diff mbox

[2/2,SRU,xenial] blk-mq: Use proper cpumask iterator

Message ID 1472500919-14631-3-git-send-email-eric.desrochers@canonical.com
State New
Headers show

Commit Message

Eric Desrochers Aug. 29, 2016, 8:01 p.m. UTC
From: Thomas Gleixner <tglx@linutronix.de>

BugLink: https://bugs.launchpad.net/bugs/1572630

queue_for_each_ctx() iterates over per_cpu variables under the assumption that
the possible cpu mask cannot have holes. That's wrong as all cpumasks can have
holes. In case there are holes the iteration ends up accessing uninitialized
memory and crashing as a result.

Replace the macro by a proper for_each_possible_cpu() loop and drop the unused
macro blk_ctx_sum() which references queue_for_each_ctx().

Reported-by: Xiong Zhou <jencce.kernel@gmail.com>
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Jens Axboe <axboe@fb.com>
(cherry picked from commit 897bb0c7f1ea82d7cc882b19790b5e1df00ffc29)
Signed-off-by: Eric Desrochers <eric.desrochers@canonical.com>
---
 block/blk-mq-sysfs.c   |  5 ++++-
 block/blk-mq.c         |  3 ++-
 include/linux/blk-mq.h | 14 --------------
 3 files changed, 6 insertions(+), 16 deletions(-)

Comments

Chris J Arges Aug. 30, 2016, 3:20 p.m. UTC | #1
On Mon, Aug 29, 2016 at 04:01:59PM -0400, Eric Desrochers wrote:
> From: Thomas Gleixner <tglx@linutronix.de>
> 
> BugLink: https://bugs.launchpad.net/bugs/1572630
> 
> queue_for_each_ctx() iterates over per_cpu variables under the assumption that
> the possible cpu mask cannot have holes. That's wrong as all cpumasks can have
> holes. In case there are holes the iteration ends up accessing uninitialized
> memory and crashing as a result.
> 
> Replace the macro by a proper for_each_possible_cpu() loop and drop the unused
> macro blk_ctx_sum() which references queue_for_each_ctx().
> 
> Reported-by: Xiong Zhou <jencce.kernel@gmail.com>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> Signed-off-by: Jens Axboe <axboe@fb.com>
> (cherry picked from commit 897bb0c7f1ea82d7cc882b19790b5e1df00ffc29)
> Signed-off-by: Eric Desrochers <eric.desrochers@canonical.com>
> ---
>  block/blk-mq-sysfs.c   |  5 ++++-
>  block/blk-mq.c         |  3 ++-
>  include/linux/blk-mq.h | 14 --------------
>  3 files changed, 6 insertions(+), 16 deletions(-)
> 
> diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
> index 1cf1878..8d41163 100644
> --- a/block/blk-mq-sysfs.c
> +++ b/block/blk-mq-sysfs.c
> @@ -413,14 +413,17 @@ static void blk_mq_sysfs_init(struct request_queue *q)
>  	struct blk_mq_hw_ctx *hctx;
>  	struct blk_mq_ctx *ctx;
>  	int i;
> +	int cpu;
> 

This is technically a 'backport' since it isn't a clean cherry-pick.
I see in the upstream patch that i was removed as we no longer have the
queue_for_each_hw_ctx loop.

--chris
>  	kobject_init(&q->mq_kobj, &blk_mq_ktype);
>  
>  	queue_for_each_hw_ctx(q, hctx, i)
>  		kobject_init(&hctx->kobj, &blk_mq_hw_ktype);
>  
> -	queue_for_each_ctx(q, ctx, i)
> +	for_each_possible_cpu(cpu) {
> +		ctx = per_cpu_ptr(q->queue_ctx, cpu);
>  		kobject_init(&ctx->kobj, &blk_mq_ctx_ktype);
> +	}
>  }
>  
>  int blk_mq_register_disk(struct gendisk *disk)
> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 32598d2..92b7800 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -1822,11 +1822,12 @@ static void blk_mq_map_swqueue(struct request_queue *q,
>  	/*
>  	 * Map software to hardware queues
>  	 */
> -	queue_for_each_ctx(q, ctx, i) {
> +	for_each_possible_cpu(i) {
>  		/* If the cpu isn't online, the cpu is mapped to first hctx */
>  		if (!cpumask_test_cpu(i, online_mask))
>  			continue;
>  
> +		ctx = per_cpu_ptr(q->queue_ctx, i);
>  		hctx = q->mq_ops->map_queue(q, i);
>  		cpumask_set_cpu(i, hctx->cpumask);
>  		ctx->index_hw = hctx->nr_ctx;
> diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
> index 7fc9296..a8756cd 100644
> --- a/include/linux/blk-mq.h
> +++ b/include/linux/blk-mq.h
> @@ -261,22 +261,8 @@ static inline void *blk_mq_rq_to_pdu(struct request *rq)
>  	for ((i) = 0; (i) < (q)->nr_hw_queues &&			\
>  	     ({ hctx = (q)->queue_hw_ctx[i]; 1; }); (i)++)
>  
> -#define queue_for_each_ctx(q, ctx, i)					\
> -	for ((i) = 0; (i) < (q)->nr_queues &&				\
> -	     ({ ctx = per_cpu_ptr((q)->queue_ctx, (i)); 1; }); (i)++)
> -
>  #define hctx_for_each_ctx(hctx, ctx, i)					\
>  	for ((i) = 0; (i) < (hctx)->nr_ctx &&				\
>  	     ({ ctx = (hctx)->ctxs[(i)]; 1; }); (i)++)
>  
> -#define blk_ctx_sum(q, sum)						\
> -({									\
> -	struct blk_mq_ctx *__x;						\
> -	unsigned int __ret = 0, __i;					\
> -									\
> -	queue_for_each_ctx((q), __x, __i)				\
> -		__ret += sum;						\
> -	__ret;								\
> -})
> -
>  #endif
> -- 
> 2.7.4
> 
> 
> -- 
> kernel-team mailing list
> kernel-team@lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
diff mbox

Patch

diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
index 1cf1878..8d41163 100644
--- a/block/blk-mq-sysfs.c
+++ b/block/blk-mq-sysfs.c
@@ -413,14 +413,17 @@  static void blk_mq_sysfs_init(struct request_queue *q)
 	struct blk_mq_hw_ctx *hctx;
 	struct blk_mq_ctx *ctx;
 	int i;
+	int cpu;
 
 	kobject_init(&q->mq_kobj, &blk_mq_ktype);
 
 	queue_for_each_hw_ctx(q, hctx, i)
 		kobject_init(&hctx->kobj, &blk_mq_hw_ktype);
 
-	queue_for_each_ctx(q, ctx, i)
+	for_each_possible_cpu(cpu) {
+		ctx = per_cpu_ptr(q->queue_ctx, cpu);
 		kobject_init(&ctx->kobj, &blk_mq_ctx_ktype);
+	}
 }
 
 int blk_mq_register_disk(struct gendisk *disk)
diff --git a/block/blk-mq.c b/block/blk-mq.c
index 32598d2..92b7800 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -1822,11 +1822,12 @@  static void blk_mq_map_swqueue(struct request_queue *q,
 	/*
 	 * Map software to hardware queues
 	 */
-	queue_for_each_ctx(q, ctx, i) {
+	for_each_possible_cpu(i) {
 		/* If the cpu isn't online, the cpu is mapped to first hctx */
 		if (!cpumask_test_cpu(i, online_mask))
 			continue;
 
+		ctx = per_cpu_ptr(q->queue_ctx, i);
 		hctx = q->mq_ops->map_queue(q, i);
 		cpumask_set_cpu(i, hctx->cpumask);
 		ctx->index_hw = hctx->nr_ctx;
diff --git a/include/linux/blk-mq.h b/include/linux/blk-mq.h
index 7fc9296..a8756cd 100644
--- a/include/linux/blk-mq.h
+++ b/include/linux/blk-mq.h
@@ -261,22 +261,8 @@  static inline void *blk_mq_rq_to_pdu(struct request *rq)
 	for ((i) = 0; (i) < (q)->nr_hw_queues &&			\
 	     ({ hctx = (q)->queue_hw_ctx[i]; 1; }); (i)++)
 
-#define queue_for_each_ctx(q, ctx, i)					\
-	for ((i) = 0; (i) < (q)->nr_queues &&				\
-	     ({ ctx = per_cpu_ptr((q)->queue_ctx, (i)); 1; }); (i)++)
-
 #define hctx_for_each_ctx(hctx, ctx, i)					\
 	for ((i) = 0; (i) < (hctx)->nr_ctx &&				\
 	     ({ ctx = (hctx)->ctxs[(i)]; 1; }); (i)++)
 
-#define blk_ctx_sum(q, sum)						\
-({									\
-	struct blk_mq_ctx *__x;						\
-	unsigned int __ret = 0, __i;					\
-									\
-	queue_for_each_ctx((q), __x, __i)				\
-		__ret += sum;						\
-	__ret;								\
-})
-
 #endif