Hi,
在 2022/10/11 21:24, John Garry 写道:
On 11/10/2022 13:10, Yu Kuai wrote:
From: Yu Kuai <yukuai3@xxxxxxxxxx>
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_rqso
blk_mq_alloc_rqs
// failed due to oom
alloc_pages_node
// set->stags[hctx_idx] is still NULL
set->tags
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 <yukuai3@xxxxxxxxxx>
We could have something like the following to ensure the drv tags are
assigned before we try to allocate the rqs (and fail), but prob not
worth the churn since it's not nice to pass &set->tags[hctx_idx]:
--->8---
-struct blk_mq_tags *blk_mq_alloc_map_and_rqs(struct blk_mq_tag_set *set,
+bool blk_mq_alloc_map_and_rqs(struct blk_mq_tag_set *set,
unsigned int hctx_idx,
- unsigned int depth)
+ unsigned int depth,
+ struct blk_mq_tags **tags)
{
- struct blk_mq_tags *tags;
int ret;
- tags = blk_mq_alloc_rq_map(set, hctx_idx, depth,
set->reserved_tags);
- if (!tags)
- return NULL;
+ *tags = blk_mq_alloc_rq_map(set, hctx_idx, depth,
set->reserved_tags);
+ if (!*tags)
+ return false;
- ret = blk_mq_alloc_rqs(set, tags, hctx_idx, depth);
+ ret = blk_mq_alloc_rqs(set, *tags, hctx_idx, depth);
if (ret) {
blk_mq_free_rq_map(tags);
+ *tags = NULL;
- return NULL;
+ return false;
}
- return tags;
+ return true;
}
static bool __blk_mq_alloc_map_and_rqs(struct blk_mq_tag_set *set,
@@ -3632,10 +3632,9 @@ static bool __blk_mq_alloc_map_and_rqs(struct
blk_mq_tag_set *set,
return true;
}
- set->tags[hctx_idx] = blk_mq_alloc_map_and_rqs(set, hctx_idx,
- set->queue_depth);
-
- return set->tags[hctx_idx];
+ return blk_mq_alloc_map_and_rqs(set, hctx_idx,
+ set->queue_depth,
+ &set->tags[hctx_idx]);
}
---8<----
So,
Reviewed-by: John Garry <john.garry@xxxxxxxxxx>
Thanks for the review, I'll send a new verion with the spelling mistakes
fixed.
Kuai
---
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) {
.
