2022-10-11 14:34:49

by Yu Kuai

[permalink] [raw]
Subject: [PATCH v2] blk-mq: fix null pointer dereference in blk_mq_clear_rq_mapping()

From: Yu Kuai <[email protected]>

Our syzkaller report a null pointer dereference, root cause is
following:

__blk_mq_alloc_map_and_rqs
set->tags[hctx_idx] = blk_mq_alloc_map_and_rqs
blk_mq_alloc_map_and_rqs
blk_mq_alloc_rqs
// failed due to oom
alloc_pages_node
// set->tags[hctx_idx] is still NULL
blk_mq_free_rqs
drv_tags = set->tags[hctx_idx];
// null pointer dereference is triggered
blk_mq_clear_rq_mapping(drv_tags, ...)

This is because commit 63064be150e4 ("blk-mq:
Add blk_mq_alloc_map_and_rqs()") merged the two steps:

1) set->tags[hctx_idx] = blk_mq_alloc_rq_map()
2) blk_mq_alloc_rqs(..., set->tags[hctx_idx])

into one step:

set->tags[hctx_idx] = blk_mq_alloc_map_and_rqs()

Since tags is not initialized yet in this case, fix the problem by
checking if tags is NULL pointer in blk_mq_clear_rq_mapping().

Fixes: 63064be150e4 ("blk-mq: Add blk_mq_alloc_map_and_rqs()")
Signed-off-by: Yu Kuai <[email protected]>
Reviewed-by: John Garry <[email protected]>
---
Changes in v2:
- fix spelling mistakes
- add review tag

block/blk-mq.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index 8070b6c10e8d..33292c01875d 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -3112,8 +3112,11 @@ static void blk_mq_clear_rq_mapping(struct blk_mq_tags *drv_tags,
struct page *page;
unsigned long flags;

- /* There is no need to clear a driver tags own mapping */
- if (drv_tags == tags)
+ /*
+ * There is no need to clear mapping if driver tags is not initialized
+ * or the mapping belongs to the driver tags.
+ */
+ if (!drv_tags || drv_tags == tags)
return;

list_for_each_entry(page, &tags->page_list, lru) {
--
2.31.1


2022-10-15 05:41:15

by Yu Kuai

[permalink] [raw]
Subject: Re: [PATCH v2] blk-mq: fix null pointer dereference in blk_mq_clear_rq_mapping()

Hi, Jens!

?? 2022/10/11 22:22, Yu Kuai ะด??:
> From: Yu Kuai <[email protected]>
>
> Our syzkaller report a null pointer dereference, root cause is
> following:
>
> __blk_mq_alloc_map_and_rqs
> set->tags[hctx_idx] = blk_mq_alloc_map_and_rqs
> blk_mq_alloc_map_and_rqs
> blk_mq_alloc_rqs
> // failed due to oom
> alloc_pages_node
> // set->tags[hctx_idx] is still NULL
> blk_mq_free_rqs
> drv_tags = set->tags[hctx_idx];
> // null pointer dereference is triggered
> blk_mq_clear_rq_mapping(drv_tags, ...)
>
> This is because commit 63064be150e4 ("blk-mq:
> Add blk_mq_alloc_map_and_rqs()") merged the two steps:
>
> 1) set->tags[hctx_idx] = blk_mq_alloc_rq_map()
> 2) blk_mq_alloc_rqs(..., set->tags[hctx_idx])
>
> into one step:
>
> set->tags[hctx_idx] = blk_mq_alloc_map_and_rqs()
>
> Since tags is not initialized yet in this case, fix the problem by
> checking if tags is NULL pointer in blk_mq_clear_rq_mapping().
>
> Fixes: 63064be150e4 ("blk-mq: Add blk_mq_alloc_map_and_rqs()")
> Signed-off-by: Yu Kuai <[email protected]>
> Reviewed-by: John Garry <[email protected]>
> ---
> Changes in v2:
> - fix spelling mistakes
> - add review tag
>
> block/blk-mq.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
Can you apply this patch?

Thanks,
Kuai

> diff --git a/block/blk-mq.c b/block/blk-mq.c
> index 8070b6c10e8d..33292c01875d 100644
> --- a/block/blk-mq.c
> +++ b/block/blk-mq.c
> @@ -3112,8 +3112,11 @@ static void blk_mq_clear_rq_mapping(struct blk_mq_tags *drv_tags,
> struct page *page;
> unsigned long flags;
>
> - /* There is no need to clear a driver tags own mapping */
> - if (drv_tags == tags)
> + /*
> + * There is no need to clear mapping if driver tags is not initialized
> + * or the mapping belongs to the driver tags.
> + */
> + if (!drv_tags || drv_tags == tags)
> return;
>
> list_for_each_entry(page, &tags->page_list, lru) {
>

2022-10-16 23:43:09

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH v2] blk-mq: fix null pointer dereference in blk_mq_clear_rq_mapping()

On Tue, 11 Oct 2022 22:22:53 +0800, Yu Kuai wrote:
> From: Yu Kuai <[email protected]>
>
> Our syzkaller report a null pointer dereference, root cause is
> following:
>
> __blk_mq_alloc_map_and_rqs
> set->tags[hctx_idx] = blk_mq_alloc_map_and_rqs
> blk_mq_alloc_map_and_rqs
> blk_mq_alloc_rqs
> // failed due to oom
> alloc_pages_node
> // set->tags[hctx_idx] is still NULL
> blk_mq_free_rqs
> drv_tags = set->tags[hctx_idx];
> // null pointer dereference is triggered
> blk_mq_clear_rq_mapping(drv_tags, ...)
>
> [...]

Applied, thanks!

[1/1] blk-mq: fix null pointer dereference in blk_mq_clear_rq_mapping()
commit: 76dd298094f484c6250ebd076fa53287477b2328

Best regards,
--
Jens Axboe