Re: [PATCHv2] md: clear MD_CLOSING when array_state_store() bails on interrupted lock

Jinpu Wang <[email protected]> Tue, 4 Aug 2026 20:44:17 +0200
Newsgroups dev.linux.lists.sashiko-reviews,org.kernel.vger.linux-raid
Message-ID <CAMGffE=gipMNJkTLoSBj26mGWsTmp2aiFKM3yRe12BdjwWftBw@mail.gmail.com>
Hi Team,

Regarding the pre-existing issue reported by the bot, it is not
related to this fix and cannot be triggered in normal cases, so I will
skip it for now.

Linux RAID folks, do you have a chance to check this patch? It fixes a
problem we hit by chance.

Thanks,
Jinpu

On Fri, Jul 17, 2026 at 7:25=E2=80=AFAM <[email protected]> wrote:
>
> Thank you for your contribution! Sashiko AI review found 1 potential issu=
e(s) to consider:
>
> Pre-existing issues:
> - [High] Calling `restart_array()` with the `mddev->lock` spinlock held c=
an sleep because it clears the disk read-only flag, which triggers a `GFP_K=
ERNEL` memory allocation for a uevent.
> --
>
> --- Patch [1]: [PATCHv2] md: clear MD_CLOSING when array_state_store() ba=
ils on interrupted lock ---
> commit 3ae9b81fbeee45b412994652c2c3b3ac1b35d177
> Author: Jack Wang <[email protected]>
>
> md: clear MD_CLOSING when array_state_store() bails on interrupted lock
>
> This commit ensures that MD_CLOSING is correctly cleared if
> mutex_lock_interruptible() fails while writing to array_state. It introdu=
ces
> a common exit label and a boolean flag to track whether the flag was actu=
ally
> set by the current thread.
>
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index d1465bcd86c81..7b7a4d9259079 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> [ ... ]
> > @@ -4696,7 +4698,7 @@ array_state_store(struct mddev *mddev, const char=
 *buf, size_t len)
> >       }
> >       err =3D mddev_lock(mddev);
> >       if (err)
> > -             return err;
> > +             goto out_clear_closing;
>
> [Severity: High]
> This is a pre-existing issue, but just above this code in array_state_sto=
re(),
> there is a fast path that can sleep while holding a spinlock.
>
> If a privileged user sets the md device to read-only using
> blockdev --setro /dev/md0 and then writes 'active' to
> /sys/block/md0/md/array_state, the code acquires the mddev->lock spinlock
> and calls restart_array():
>
> drivers/md/md.c:array_state_store() {
>     ...
>     spin_lock(&mddev->lock);
>     if (st =3D=3D active) {
>         restart_array(mddev);
>     ...
> }
>
> restart_array() unconditionally calls set_disk_ro() to ensure the disk is
> read-write:
>
> drivers/md/md.c:restart_array() {
>     ...
>     set_disk_ro(disk, 0);
>     ...
> }
>
> Because the disk was set to read-only, set_disk_ro() clears the flag and
> triggers a uevent:
>
> block/genhd.c:set_disk_ro() {
>     ...
>     set_disk_ro_uevent(disk, read_only);
>     ...
> }
>
> This calls into kobject_uevent_env(), which performs an unconditional
> GFP_KERNEL allocation:
>
> lib/kobject_uevent.c:kobject_uevent_env() {
>     ...
>     devpath =3D kobject_get_path(kobj, GFP_KERNEL);
>     ...
> }
>
> Can this sequence cause a sleep-in-atomic bug and potentially lead to a
> deadlock or panic?
>
> --
> Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260717051233.38=
[email protected]?part=3D1