Re: [PATCH] Fix array overflow in CFQ

Jens Axboe
2010-10-19T10:01:43Z

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