Re: BUG: NULL pointer dereferenced within __blk_rq_map_sg

[Ming Lei <ming.lei@redhat.com>]

On Wed, Feb 12, 2025 at 04:24:43PM -0700, Cheyenne Wills wrote:
> On Tue, Feb 11, 2025 at 8:29 AM Ming Lei <ming.lei@redhat.com> wrote:
> >
> > On Tue, Feb 11, 2025 at 08:13:16PM +0800, Ming Lei wrote:
> > > On Fri, Feb 07, 2025 at 07:09:39PM -0700, Cheyenne Wills wrote:
> > > > While I was setting up to test with linux 6.14-rc1 (under Xen), I ran
> > > > into a consistent NULL ptr dereference within __blk_rq_map_sg when
> > > > booting the system.
> > > >
> > > > Using git bisect I was able to narrow down the "bad" commit to:
> > > >
> > > > block: add a dma mapping iterator (b7175e24d6acf79d9f3af9ce9d3d50de1fa748ec)
> > > >
> > > > Building a kernel with the parent commit
> > > > (2caca8fc7aad9ea9a6ea3ed26ed146b1e5f06fab) using the same .config does
> > > > not fail.
> > > >
> > > > Following is the console log showing the error as well as the Xen
> > > > (libvirt) configuration for the guest that I'm using.
> > > >
> > > > Please let me know if there is any additional information that I can provide.
> > >
> > > Can you test the following patch?
> > >
> >
> > Please try the revised one:
> >
> >
> > diff --git a/block/blk-merge.c b/block/blk-merge.c
> > index 15cd231d560c..a66d087a6b55 100644
> > --- a/block/blk-merge.c
> > +++ b/block/blk-merge.c
> > @@ -493,7 +493,7 @@ static bool blk_map_iter_next(struct request *req,
> >                 return true;
> >         }
> >
> > -       if (!iter->iter.bi_size)
> > +       if (!iter->bio || !iter->iter.bi_size)
> >                 return false;
> >
> >         bv = mp_bvec_iter_bvec(iter->bio->bi_io_vec, iter->iter);
> > @@ -514,6 +514,8 @@ static bool blk_map_iter_next(struct request *req,
> >                         if (!iter->bio->bi_next)
> >                                 break;
> >                         iter->bio = iter->bio->bi_next;
> > +                       if (!iter->bio)
> > +                               break;
> >                         iter->iter = iter->bio->bi_iter;
> >                 }
> >
> >
> >
> >
> > Thanks,
> > Ming
> >
> 
> Still getting a BUG at the same location.
> 
> I was able to capture the BUG using a xen gdbsx / gdb session (the
> offending instruction is a mov  0x28(%rdx),%r13d and the bug is that
> %rdx is zero. -- break *__blk_rq_map_sg+0x5e if $rdx == 0)
> 
> It appears in __blk_rq_map_sg that the rq->bio is NULL at the start of
> the routine.

Yeah, turns out oops is triggered in initializing req_iterator for
discard req, and the following patch should be enough:


diff --git a/block/blk-merge.c b/block/blk-merge.c
index 15cd231d560c..9d7e87052882 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -556,11 +556,13 @@ 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;
 
+	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,


Thanks,
Ming