Re: [PATCH] md: avoid block autoload while importing rdev under reconfig_mutex
From: yu kuai
Date: Wed Nov 11 2026 - 05:40:32 EST
Hi,
在 2026/11/11 14:32, 王继晨 写道:
>> 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.
I still don't understand how is the ABBA deadlock possible, md_alloc() hold
reconfig_mutex before add_disk() and kobject_add() for mddev->kobj, there is no
concurrent context that can hold reconfig_mutex before md_alloc() hold it.
>
> 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.
For this test case, you can just use mddev private lockdep class to initialize
reconfig_mutex, or if there is still other lockdep wanings please attach the
report.
>
> 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
>>
--
Thanks,
Kuai