2019-08-07 17:44:04

by Paolo Valente

[permalink] [raw]
Subject: [BUGFIX 0/1] handle NULL return value by bfq_init_rq()

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


2019-08-07 17:44:21

by Paolo Valente

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

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 <[email protected]>
Reported-by: Hsin-Yi Wang <[email protected]>
Cc: Hsin-Yi Wang <[email protected]>
Cc: Nicolas Boichat <[email protected]>
Cc: Doug Anderson <[email protected]>
Signed-off-by: Paolo Valente <[email protected]>
---
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

2019-08-07 19:22:21

by Guenter Roeck

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

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 <[email protected]>
> Reported-by: Hsin-Yi Wang <[email protected]>
> Cc: Hsin-Yi Wang <[email protected]>
> Cc: Nicolas Boichat <[email protected]>
> Cc: Doug Anderson <[email protected]>
> Signed-off-by: Paolo Valente <[email protected]>

Reviewed-by: Guenter Roeck <[email protected]>

> ---
> 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
>

2019-08-08 13:33:18

by Jens Axboe

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

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