From: Ming Lei ming.lei@redhat.com
mainline inclusion from mainline-v5.13 commit 2e315dc07df009c3e29d6926871f62a30cfae394 category: bugfix bugzilla: 34280, https://gitee.com/openeuler/kernel/issues/I4AKY4 CVE: NA
-----------------------------------------------
Grab rq->refcount before calling ->fn in blk_mq_tagset_busy_iter(), and this way will prevent the request from being re-used when ->fn is running. The approach is same as what we do during handling timeout.
Fix request use-after-free(UAF) related with completion race or queue releasing:
- If one rq is referred before rq->q is frozen, then queue won't be frozen before the request is released during iteration.
- If one rq is referred after rq->q is frozen, refcount_inc_not_zero() will return false, and we won't iterate over this request.
However, still one request UAF not covered: refcount_inc_not_zero() may read one freed request, and it will be handled in next patch.
Tested-by: John Garry john.garry@huawei.com Reviewed-by: Christoph Hellwig hch@lst.de Reviewed-by: Bart Van Assche bvanassche@acm.org Signed-off-by: Ming Lei ming.lei@redhat.com Link: https://lore.kernel.org/r/20210511152236.763464-3-ming.lei@redhat.com Signed-off-by: Jens Axboe axboe@kernel.dk
Conflicts: - commit ea4f995ee8b8 ("blk-mq: cache request hardware queue mapping") is not backported, rq->mq_hctx doesn't exist, use blk_mq_map_queue() instead. - commit 602380d28e28 ("blk-mq: add blk_mq_all_tag_iter") is not backported, bt_tags_iter_data->flags doesn't exist, use 'reserved'. - commit 22f614bc0f37("blk-mq: fix blk_mq_all_tag_iter") is not backported, we won't use static_rqs in bt_tags_iter(). Signed-off-by: Yu Kuai yukuai3@huawei.com Reviewed-by: Hou Tao houtao1@huawei.com Signed-off-by: Yang Yingliang yangyingliang@huawei.com --- block/blk-mq-tag.c | 25 ++++++++++++++++++++++--- block/blk-mq.c | 13 +++++++++---- block/blk-mq.h | 1 + 3 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/block/blk-mq-tag.c b/block/blk-mq-tag.c index c8997e5c30f8a..dcf65c9b767ba 100644 --- a/block/blk-mq-tag.c +++ b/block/blk-mq-tag.c @@ -219,6 +219,16 @@ struct bt_iter_data { bool reserved; };
+static struct request *blk_mq_find_and_get_req(struct blk_mq_tags *tags, + unsigned int bitnr) +{ + struct request *rq = tags->rqs[bitnr]; + + if (!rq || !refcount_inc_not_zero(&rq->ref)) + return NULL; + return rq; +} + static bool bt_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data) { struct bt_iter_data *iter_data = data; @@ -235,8 +245,13 @@ static bool bt_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data) * We can hit rq == NULL here, because the tagging functions * test and set the bit before assining ->rqs[]. */ - if (rq && rq->q == hctx->queue) + rq = blk_mq_find_and_get_req(tags, bitnr); + if (!rq) + return true; + + if (rq->q == hctx->queue) iter_data->fn(hctx, rq, iter_data->data, reserved); + blk_mq_put_rq_ref(rq, hctx); return true; }
@@ -274,9 +289,13 @@ static bool bt_tags_iter(struct sbitmap *bitmap, unsigned int bitnr, void *data) * We can hit rq == NULL here, because the tagging functions * test and set the bit before assining ->rqs[]. */ - rq = tags->rqs[bitnr]; - if (rq && blk_mq_request_started(rq)) + rq = blk_mq_find_and_get_req(tags, bitnr); + if (!rq) + return true; + + if (blk_mq_request_started(rq)) iter_data->fn(rq, iter_data->data, reserved); + blk_mq_put_rq_ref(rq, blk_mq_map_queue(rq->q, rq->mq_ctx->cpu));
return true; } diff --git a/block/blk-mq.c b/block/blk-mq.c index 611c044a5a36c..15da63e06fbbb 100644 --- a/block/blk-mq.c +++ b/block/blk-mq.c @@ -857,6 +857,14 @@ static bool blk_mq_req_expired(struct request *rq, unsigned long *next) return false; }
+void blk_mq_put_rq_ref(struct request *rq, struct blk_mq_hw_ctx *hctx) +{ + if (is_flush_rq(rq, hctx)) + rq->end_io(rq, 0); + else if (refcount_dec_and_test(&rq->ref)) + __blk_mq_free_request(rq); +} + static void blk_mq_check_expired(struct blk_mq_hw_ctx *hctx, struct request *rq, void *priv, bool reserved) { @@ -890,10 +898,7 @@ static void blk_mq_check_expired(struct blk_mq_hw_ctx *hctx, if (blk_mq_req_expired(rq, next)) blk_mq_rq_timed_out(rq, reserved);
- if (is_flush_rq(rq, hctx)) - rq->end_io(rq, 0); - else if (refcount_dec_and_test(&rq->ref)) - __blk_mq_free_request(rq); + blk_mq_put_rq_ref(rq, hctx); }
static void blk_mq_timeout_work(struct work_struct *work) diff --git a/block/blk-mq.h b/block/blk-mq.h index debc646e1bed8..1fc3debe69f94 100644 --- a/block/blk-mq.h +++ b/block/blk-mq.h @@ -45,6 +45,7 @@ void blk_mq_flush_busy_ctxs(struct blk_mq_hw_ctx *hctx, struct list_head *list); bool blk_mq_get_driver_tag(struct request *rq); struct request *blk_mq_dequeue_from_ctx(struct blk_mq_hw_ctx *hctx, struct blk_mq_ctx *start); +void blk_mq_put_rq_ref(struct request *rq, struct blk_mq_hw_ctx *hctx);
/* * Internal helpers for allocating/freeing the request map