[BUGFIX 0/1] handle NULL return value by bfq_init_rq()

[BUGFIX 0/1] handle NULL return value by bfq_init_rq()

[Paolo Valente]

Hi Jens,
this is a hopefully complete version of the fix proposed by Guenter [1].

Thanks,
Paolo

[1] https://lkml.org/lkml/2019/7/22/824

Paolo Valente (1):
  block, bfq: handle NULL return value by bfq_init_rq()

 block/bfq-iosched.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

--
2.20.1

[BUGFIX 1/1] block, bfq: handle NULL return value by bfq_init_rq()

[Paolo Valente]

As reported in [1], the call bfq_init_rq(rq) may return NULL in case
of OOM (in particular, if rq->elv.icq is NULL because memory
allocation failed in failed in ioc_create_icq()).

This commit handles this circumstance.

[1] https://lkml.org/lkml/2019/7/22/824

Reported-by: Guenter Roeck <linux@roeck-us.net>
Reported-by: Hsin-Yi Wang <hsinyi@google.com>
Cc: Hsin-Yi Wang <hsinyi@google.com>
Cc: Nicolas Boichat <drinkcat@chromium.org>
Cc: Doug Anderson <dianders@chromium.org>
Signed-off-by: Paolo Valente <paolo.valente@linaro.org>
---
 block/bfq-iosched.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
index 586fcfe227ea..32686300d89b 100644
--- a/block/bfq-iosched.c
+++ b/block/bfq-iosched.c
@@ -2250,9 +2250,14 @@ static void bfq_request_merged(struct request_queue *q, struct request *req,
 	    blk_rq_pos(container_of(rb_prev(&req->rb_node),
 				    struct request, rb_node))) {
 		struct bfq_queue *bfqq = bfq_init_rq(req);
-		struct bfq_data *bfqd = bfqq->bfqd;
+		struct bfq_data *bfqd;
 		struct request *prev, *next_rq;
 
+		if (!bfqq)
+			return;
+
+		bfqd = bfqq->bfqd;
+
 		/* Reposition request in its sort_list */
 		elv_rb_del(&bfqq->sort_list, req);
 		elv_rb_add(&bfqq->sort_list, req);
@@ -2299,6 +2304,9 @@ static void bfq_requests_merged(struct request_queue *q, struct request *rq,
 	struct bfq_queue *bfqq = bfq_init_rq(rq),
 		*next_bfqq = bfq_init_rq(next);
 
+	if (!bfqq)
+		return;
+
 	/*
 	 * If next and rq belong to the same bfq_queue and next is older
 	 * than rq, then reposition rq in the fifo (by substituting next
@@ -5436,12 +5444,12 @@ static void bfq_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq,
 
 	spin_lock_irq(&bfqd->lock);
 	bfqq = bfq_init_rq(rq);
-	if (at_head || blk_rq_is_passthrough(rq)) {
+	if (!bfqq || at_head || blk_rq_is_passthrough(rq)) {
 		if (at_head)
 			list_add(&rq->queuelist, &bfqd->dispatch);
 		else
 			list_add_tail(&rq->queuelist, &bfqd->dispatch);
-	} else { /* bfqq is assumed to be non null here */
+	} else {
 		idle_timer_disabled = __bfq_insert_request(bfqd, rq);
 		/*
 		 * Update bfqq, because, if a queue merge has occurred
-- 
2.20.1

Re: [BUGFIX 1/1] block, bfq: handle NULL return value by bfq_init_rq()

[Guenter Roeck]

On 8/7/19 10:21 AM, Paolo Valente wrote:
> As reported in [1], the call bfq_init_rq(rq) may return NULL in case
> of OOM (in particular, if rq->elv.icq is NULL because memory
> allocation failed in failed in ioc_create_icq()).
> 
> This commit handles this circumstance.
> 
> [1] https://lkml.org/lkml/2019/7/22/824
> 
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Reported-by: Hsin-Yi Wang <hsinyi@google.com>
> Cc: Hsin-Yi Wang <hsinyi@google.com>
> Cc: Nicolas Boichat <drinkcat@chromium.org>
> Cc: Doug Anderson <dianders@chromium.org>
> Signed-off-by: Paolo Valente <paolo.valente@linaro.org>

Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> ---
>   block/bfq-iosched.c | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/block/bfq-iosched.c b/block/bfq-iosched.c
> index 586fcfe227ea..32686300d89b 100644
> --- a/block/bfq-iosched.c
> +++ b/block/bfq-iosched.c
> @@ -2250,9 +2250,14 @@ static void bfq_request_merged(struct request_queue *q, struct request *req,
>   	    blk_rq_pos(container_of(rb_prev(&req->rb_node),
>   				    struct request, rb_node))) {
>   		struct bfq_queue *bfqq = bfq_init_rq(req);
> -		struct bfq_data *bfqd = bfqq->bfqd;
> +		struct bfq_data *bfqd;
>   		struct request *prev, *next_rq;
>   
> +		if (!bfqq)
> +			return;
> +
> +		bfqd = bfqq->bfqd;
> +
>   		/* Reposition request in its sort_list */
>   		elv_rb_del(&bfqq->sort_list, req);
>   		elv_rb_add(&bfqq->sort_list, req);
> @@ -2299,6 +2304,9 @@ static void bfq_requests_merged(struct request_queue *q, struct request *rq,
>   	struct bfq_queue *bfqq = bfq_init_rq(rq),
>   		*next_bfqq = bfq_init_rq(next);
>   
> +	if (!bfqq)
> +		return;
> +
>   	/*
>   	 * If next and rq belong to the same bfq_queue and next is older
>   	 * than rq, then reposition rq in the fifo (by substituting next
> @@ -5436,12 +5444,12 @@ static void bfq_insert_request(struct blk_mq_hw_ctx *hctx, struct request *rq,
>   
>   	spin_lock_irq(&bfqd->lock);
>   	bfqq = bfq_init_rq(rq);
> -	if (at_head || blk_rq_is_passthrough(rq)) {
> +	if (!bfqq || at_head || blk_rq_is_passthrough(rq)) {
>   		if (at_head)
>   			list_add(&rq->queuelist, &bfqd->dispatch);
>   		else
>   			list_add_tail(&rq->queuelist, &bfqd->dispatch);
> -	} else { /* bfqq is assumed to be non null here */
> +	} else {
>   		idle_timer_disabled = __bfq_insert_request(bfqd, rq);
>   		/*
>   		 * Update bfqq, because, if a queue merge has occurred
> 

Re: [BUGFIX 0/1] handle NULL return value by bfq_init_rq()

[Jens Axboe]

On 8/7/19 10:21 AM, Paolo Valente wrote:
> Hi Jens,
> this is a hopefully complete version of the fix proposed by Guenter [1].
> 
> Thanks,
> Paolo
> 
> [1] https://lkml.org/lkml/2019/7/22/824
> 
> Paolo Valente (1):
>    block, bfq: handle NULL return value by bfq_init_rq()
> 
>   block/bfq-iosched.c | 14 +++++++++++---
>   1 file changed, 11 insertions(+), 3 deletions(-)

Applied, thanks.

-- 
Jens Axboe