Re: [PATCH] Fix array overflow in CFQ

[Jens Axboe <axboe@kernel.dk>]

On 2010-10-19 11:10, Andi Kleen wrote:
> From: Andi Kleen <ak@linux.intel.com>
> 
> gcc 4.5 complains when compiling a recent rc with
> 
> linux/block/cfq-iosched.c: In function ‘cfq_dispatch_requests’:
> linux/block/cfq-iosched.c:2156:3: warning: array subscript is above array bounds
> 
> and it is right:
> 
>  slice = group_slice * count /
>                 max_t(unsigned, cfqg->busy_queues_avg[cfqd->serving_prio],
>                       cfq_group_busy_queues_wl(cfqd->serving_prio, cfqd, cfqg));
> 
> busy_queues_avg can be indexed by this enum
> 
> enum wl_prio_t {
>         BE_WORKLOAD = 0,
>         RT_WORKLOAD = 1,
>         IDLE_WORKLOAD = 2,
> };
> 
> in cfqd->serving_prio, but is only declared as
> 
> unsigned int busy_queues_avg[2];
> 
> which is clearly off by one. Fix this here.

Indeed, that is definitely buggy. ->service_trees[][] looks buggy, too.
WTF?!

diff --git a/block/cfq-iosched.c b/block/cfq-iosched.c
index 9eba291..8ce9f52 100644
--- a/block/cfq-iosched.c
+++ b/block/cfq-iosched.c
@@ -160,6 +160,7 @@ enum wl_prio_t {
 	BE_WORKLOAD = 0,
 	RT_WORKLOAD = 1,
 	IDLE_WORKLOAD = 2,
+	CFQ_PRIO_NR,
 };
 
 /*
@@ -168,7 +169,8 @@ enum wl_prio_t {
 enum wl_type_t {
 	ASYNC_WORKLOAD = 0,
 	SYNC_NOIDLE_WORKLOAD = 1,
-	SYNC_WORKLOAD = 2
+	SYNC_WORKLOAD = 2,
+	CFQ_TYPE_NR,
 };
 
 /* This is per cgroup per device grouping structure */
@@ -185,12 +187,12 @@ struct cfq_group {
 	int nr_cfqq;
 
 	/* Per group busy queus average. Useful for workload slice calc. */
-	unsigned int busy_queues_avg[2];
+	unsigned int busy_queues_avg[CFQ_PRIO_NR];
 	/*
 	 * rr lists of queues with requests, onle rr for each priority class.
 	 * Counts are embedded in the cfq_rb_root
 	 */
-	struct cfq_rb_root service_trees[2][3];
+	struct cfq_rb_root service_trees[CFQ_PRIO_NR][CFQ_TYPE_NR];
 	struct cfq_rb_root service_tree_idle;
 
 	unsigned long saved_workload_slice;

-- 
Jens Axboe