| Message ID | 20250214084638.58437-1-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | block: fix NULL pointer dereferenced within __blk_rq_map_sg | expand |
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.
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
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,
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(-)