[PATCH] block: fix NULL pointer dereferenced within __blk_rq_map_sg

[PATCH] block: fix NULL pointer dereferenced within __blk_rq_map_sg

[Ming Lei]

Discard request may use special payload only and doesn't have bio
attached, so the request iterator has to be initialized from valid
req->bio, otherwise NULL pointer dereferenced is triggered.

Cc: Christoph Hellwig <hch@lst.de>
Reported-and-tested-by: Cheyenne Wills <cheyenne.wills@gmail.com>
Fixes: b7175e24d6ac ("block: add a dma mapping iterator")
Signed-off-by: Ming Lei <ming.lei@redhat.com>
---
 block/blk-merge.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/block/blk-merge.c b/block/blk-merge.c
index b55c52a42303..48a96aed15b6 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -556,11 +556,14 @@ int __blk_rq_map_sg(struct request_queue *q, struct request *rq,
 {
 	struct req_iterator iter = {
 		.bio	= rq->bio,
-		.iter	= rq->bio->bi_iter,
 	};
 	struct phys_vec vec;
 	int nsegs = 0;
 
+	/* discard request may not have bio attached */
+	if (iter.bio)
+		iter.iter = iter.bio->bi_iter;
+
 	while (blk_map_iter_next(rq, &iter, &vec)) {
 		*last_sg = blk_next_sg(last_sg, sglist);
 		sg_set_page(*last_sg, phys_to_page(vec.paddr), vec.len,
-- 
2.47.1

Re: [PATCH] block: fix NULL pointer dereferenced within __blk_rq_map_sg

[Christoph Hellwig]

On Fri, Feb 14, 2025 at 04:46:38PM +0800, Ming Lei wrote:
> Discard request may use special payload only and doesn't have bio
> attached, so the request iterator has to be initialized from valid
> req->bio, otherwise NULL pointer dereferenced is triggered.

So while the code changes here look good to me, the commit message is
wrong.  discard requests always have at least one bio attached, so we're
not going to hit this condition.  Discard requests also aren't even
handled by the function in Cheyenne's report.  I'm pretty sure this is
a flush request, as these are the only non-passthrough requests without
a bio.

> +	/* discard request may not have bio attached */
> +	if (iter.bio)
> +		iter.iter = iter.bio->bi_iter;

Same for the comment.

Re: [PATCH] block: fix NULL pointer dereferenced within __blk_rq_map_sg

[Ming Lei]

On Fri, Feb 14, 2025 at 03:10:10PM +0100, Christoph Hellwig wrote:
> On Fri, Feb 14, 2025 at 04:46:38PM +0800, Ming Lei wrote:
> > Discard request may use special payload only and doesn't have bio
> > attached, so the request iterator has to be initialized from valid
> > req->bio, otherwise NULL pointer dereferenced is triggered.
> 
> So while the code changes here look good to me, the commit message is
> wrong.  discard requests always have at least one bio attached, so we're
> not going to hit this condition.  Discard requests also aren't even
> handled by the function in Cheyenne's report.  I'm pretty sure this is
> a flush request, as these are the only non-passthrough requests without
> a bio.
> 
> > +	/* discard request may not have bio attached */
> > +	if (iter.bio)
> > +		iter.iter = iter.bio->bi_iter;
> 
> Same for the comment.
 
You are right, it should be the flush internal request, even though
mapping discard request may not need bio, I will fix the commit log and
comment.


Thanks,
Ming