[PATCH] blk-mq: always clear rq->bio in blk_complete_request()

[Michael Liang]

Commit ab3e1d3bbab9 ("block: allow end_io based requests in the
completion batch handling") changed blk_complete_request() so that
rq->bio and rq->__data_len are only cleared when ->end_io is NULL.

This conditional clearing is incorrect. The block layer guarantees that
all bios attached to the request are fully completed and released before
blk_complete_request() is called. Leaving rq->bio pointing to already
completed bios results in stale pointers that may be reused immediately
by a bioset allocator.

Stale rq->bio values have been observed to cause double-initialization
of cloned bios in request-based device-mapper targets, leading to
use-after-free and double-free scenarios. One such case occurs when
using dm-multipath on top of a PCIe NVMe namespace, where cloned request
bios are freed during blk_complete_request(), but rq->bio is left
intact. Subsequent clone teardown then attempts to free the same bios
again via blk_rq_unprep_clone(). Below is the codepath of such double-free:
nvme_pci_complete_batch()
    nvme_complete_batch()
        blk_mq_end_request_batch()
            blk_complete_request() // called on a DM-target clone req
                bio_endio() // 1st free of all bios of the clone req
                ...
            rq->end_io() // calls end_clone_request() since @rq is a clone req
                dm_compelte_request(tio->orig)
                    dm_softirq_done() // Note this actually defers to softirq context
                        dm_done()
                            dm_end_request() // end the clone request
                                blk_rq_unprep_clone() // 2nd free of BIOs on the clone req

There is no valid case where rq->bio may still reference live bios at
this point. Clear rq->bio and rq->__data_len unconditionally to avoid
leaking stale pointer state across completions.

Fixes: ab3e1d3bbab9 ("block: allow end_io based requests in the
completion batch handling")

Signed-off-by: Michael Liang <mliang@xxxxxxxxxxxxxxx>
---
 block/blk-mq.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/block/blk-mq.c b/block/blk-mq.c
index d626d32f6e57..b8b9ca2200e4 100644
--- a/block/blk-mq.c
+++ b/block/blk-mq.c
@@ -905,10 +905,8 @@ static void blk_complete_request(struct request *req)
 	 * can find how many bytes remain in the request
 	 * later.
 	 */
-	if (!req->end_io) {
-		req->bio = NULL;
-		req->__data_len = 0;
-	}
+	req->bio = NULL;
+	req->__data_len = 0;
 }
 
 /**
-- 
2.34.1