Re: [PATCH] md: avoid block autoload while importing rdev under reconfig_mutex

[王继晨]

> From: "yu kuai"<yukuai@xxxxxxx>
> Date:  Sun, Nov 8, 2026, 11:18
> Subject:  Re: [PATCH] md: avoid block autoload while importing rdev under reconfig_mutex
> To: "Wang Jichen"<wangjichen@xxxxxxxxxxx>, "Jens Axboe"<axboe@xxxxxxxxx>, "Song Liu"<song@xxxxxxxxxx>, "Li Nan"<magiclinan@xxxxxxxxxxxxxx>, "Xiao Ni"<xiao@xxxxxxxxxx>, <linux-block@xxxxxxxxxxxxxxx>, <linux-kernel@xxxxxxxxxxxxxxx>, <linux-raid@xxxxxxxxxxxxxxx>, "yu kuai"<yukuai@xxxxxxx>
> Cc: <syzbot+cd254889291ef8666036@xxxxxxxxxxxxxxxxxxxxxxxxx>
> Hi,
> 
> 在 2026/11/3 19:55, Wang Jichen 写道:
> > From: wangjichen <wangjichen@xxxxxxxxxxx>
> >
> > md_import_device() runs with mddev->reconfig_mutex held from md_ioctl().
> > Opening a member disk via bdev_file_open_by_dev() triggers legacy block
> > autoload (blk_probe_dev), which takes major_names_lock. md_probe() can
> > run under that lock and call md_alloc(), which takes disks_mutex and
> > reconfig_mutex — a lockdep ABBA with the ioctl path.
> 
> Is this a real deadlock? mddev is already existed from md_import_device(),
> while md_alloc() just allocate a new mddev, so the reconfig_mtuex is not
> the same.

Hi Kuai,

Thanks for the question.

You're right that md_ioctl() holds reconfig_mutex on the existing array
mddev, while md_alloc() takes reconfig_mutex on a newly allocated mddev —
they are different per-device mutex instances, not the same lock.

The syzbot/lockdep report is about a lock *class* cycle, not a
single-mddev self-deadlock. Lockdep treats all &mddev->reconfig_mutex as
one class, so the dependency it reports is:

  reconfig_mutex -> major_names_lock -> disks_mutex -> reconfig_mutex

Concrete race (syzbot extid cd254889291ef8666036, C repro on QEMU):

- Thread doing ADD_NEW_DISK on array A holds A's reconfig_mutex, then
  md_import_device() opens a member via bdev_file_open_by_dev(), which
  with legacy autoload calls blk_probe_dev() and takes major_names_lock.

- Concurrent open of an unregistered md node takes major_names_lock in
  blk_probe_dev(), runs md_probe() -> md_alloc() for device B, takes
  disks_mutex, then B's reconfig_mutex.

Without lockdep, the same ABBA ordering between threads can still hang;
the syzbot report includes the standard CPU0/CPU1 "Possible unsafe
locking scenario" text for this chain.

For context on when reconfig_mutex entered the md_alloc() path: the cycle
became visible after the fix for the half-initialised kobj window:

  ca39f7502425 ("md: fix mddev->kobj lifetime")
  d1472ca2ca5d ("md: don't hand out the array before md_alloc() has added
                mddev->kobj")

The latter holds reconfig_mutex across add_disk() and kobject_add() so
ioctls wait instead of touching an uninitialised mddev->kobj (Fixes:
ca39f7502425). That closes the kobject race but completes this lockdep
ring with ioctl + legacy autoload on the other side.

This patch does not revert that behaviour. It only breaks the ioctl-side
edge by not triggering legacy block autoload while reconfig_mutex is
held: md_import_device() uses bdev_file_open_by_dev_autoload(..., false).
Member disks are expected to already exist for normal mdadmin usage; the
syz repro still gets -ENXIO for bogus minors, but on a fixed kernel there
is no lockdep warning (Tested with syzbot's C repro on QEMU, 6628e7d4
bzImage, before/after).

If maintainers prefer a block-layer approach instead (e.g. not calling
md probe callbacks under major_names_lock in blk_probe_dev()), I'm happy
to respin.

Thanks,
Jichen

> 
> >
> > Add bdev_file_open_by_dev_autoload() and use autoload=false from
> > md_import_device(); member devices must already be present in the
> > block layer (normal mdadmin usage).
> >
> > Link: https://syzbot.org/bug?extid=cd254889291ef8666036
> > Reported-by: syzbot+cd254889291ef8666036@xxxxxxxxxxxxxxxxxxxxxxxxx
> > Tested-by: Wang Jichen <wangjichen@xxxxxxxxxxx>
> > Signed-off-by: Wang Jichen <wangjichen@xxxxxxxxxxx>
> > ---
> >   block/bdev.c           | 13 ++++++++++---
> >   drivers/md/md.c        |  8 ++++++--
> >   include/linux/blkdev.h |  2 ++
> >   3 files changed, 18 insertions(+), 5 deletions(-)
> >
> > diff --git a/block/bdev.c b/block/bdev.c
> > index f20f91b31..e78936556 100644
> > --- a/block/bdev.c
> > +++ b/block/bdev.c
> > @@ -1076,8 +1076,8 @@ static unsigned blk_to_file_flags(blk_mode_t mode)
> >           return flags;
> >   }
> >   
> > -struct file *bdev_file_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
> > -                                   const struct blk_holder_ops *hops)
> > +struct file *bdev_file_open_by_dev_autoload(dev_t dev, blk_mode_t mode,
> > +                void *holder, const struct blk_holder_ops *hops, bool autoload)
> >   {
> >           struct file *bdev_file;
> >           struct block_device *bdev;
> > @@ -1088,7 +1088,7 @@ struct file *bdev_file_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
> >           if (ret)
> >                   return ERR_PTR(ret);
> >   
> > -        bdev = blkdev_get_no_open(dev, true);
> > +        bdev = blkdev_get_no_open(dev, autoload);
> >           if (!bdev)
> >                   return ERR_PTR(-ENXIO);
> >   
> > @@ -1110,6 +1110,13 @@ struct file *bdev_file_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
> >           }
> >           return bdev_file;
> >   }
> > +EXPORT_SYMBOL(bdev_file_open_by_dev_autoload);
> > +
> > +struct file *bdev_file_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
> > +                                   const struct blk_holder_ops *hops)
> > +{
> > +        return bdev_file_open_by_dev_autoload(dev, mode, holder, hops, true);
> > +}
> >   EXPORT_SYMBOL(bdev_file_open_by_dev);
> >   
> >   struct file *bdev_file_open_by_path(const char *path, blk_mode_t mode,
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index 03e35386a..d3717e2e8 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -3858,9 +3858,13 @@ static struct md_rdev *md_import_device(dev_t newdev, int super_format, int supe
> >           if (err)
> >                   goto out_clear_rdev;
> >   
> > -        rdev->bdev_file = bdev_file_open_by_dev(newdev,
> > +        /*
> > +         * md_ioctl() holds reconfig_mutex; legacy autoload in bdev open takes
> > +         * major_names_lock via blk_probe_dev() and can deadlock with md_probe().
> > +         */
> > +        rdev->bdev_file = bdev_file_open_by_dev_autoload(newdev,
> >                           BLK_OPEN_READ | BLK_OPEN_WRITE,
> > -                        super_format == -2 ? &claim_rdev : rdev, NULL);
> > +                        super_format == -2 ? &claim_rdev : rdev, NULL, false);
> >           if (IS_ERR(rdev->bdev_file)) {
> >                   pr_warn("md: could not open device unknown-block(%u,%u).\n",
> >                           MAJOR(newdev), MINOR(newdev));
> > diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
> > index 0c2010230..e5121101a 100644
> > --- a/include/linux/blkdev.h
> > +++ b/include/linux/blkdev.h
> > @@ -1699,6 +1699,8 @@ struct blk_holder_ops {
> >   
> >   struct file *bdev_file_open_by_dev(dev_t dev, blk_mode_t mode, void *holder,
> >                   const struct blk_holder_ops *hops);
> > +struct file *bdev_file_open_by_dev_autoload(dev_t dev, blk_mode_t mode,
> > +                void *holder, const struct blk_holder_ops *hops, bool autoload);
> >   struct file *bdev_file_open_by_path(const char *path, blk_mode_t mode,
> >                   void *holder, const struct blk_holder_ops *hops);
> >   int bd_prepare_to_claim(struct block_device *bdev, void *holder,
> 
> -- 
> Thanks,
> Kuai
>