mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] md: don't hand out the array before md_alloc() has added mddev->kobj
@ 2026-10-04  5:57 Yogesh Gaur
  2026-10-05 15:47 ` Logan Gunthorpe
  2026-10-05 17:05 ` yu kuai
  0 siblings, 2 replies; 4+ messages in thread
From: Yogesh Gaur @ 2026-10-04  5:57 UTC (permalink / raw)
  To: Song Liu, Yu Kuai
  Cc: linux-raid, linux-kernel, Li Nan, Xiao Ni, Christoph Hellwig,
	Hannes Reinecke, Logan Gunthorpe, Yogesh Gaur,
	syzbot+95eeb4ada2349a2170ea, stable

md_alloc() publishes the gendisk before it is done setting the mddev up:

	disk->private_data = mddev;
	...
	error = add_disk(disk);
	if (error)
		goto out_put_disk;

	kobject_init(&mddev->kobj, &md_ktype);
	error = kobject_add(&mddev->kobj, &disk_to_dev(disk)->kobj, "%s", "md");

add_disk() makes /dev/mdN openable, and md_open() only refuses the open
when MD_CLOSING is set.  mddev comes from mddev_alloc() and is zeroed,
so anything that gets in between add_disk() and kobject_add() sees an
mddev->kobj that has never been through kobject_init() - no ktype, no
kref, state_initialized clear.

syzbot opens the array in that window and issues ADD_NEW_DISK:

  kobject: '(null)' (ffff8880120640f0): is not initialized, yet kobject_get() is being called.
  WARNING: lib/kobject.c:642 at kobject_add_internal+0xea/0xcd0 lib/kobject.c:225
   kobject_add_varg lib/kobject.c:374 [inline]
   kobject_add+0x163/0x240 lib/kobject.c:426
   bind_rdev_to_array+0x80c/0xdd0 drivers/md/md.c:2621
   md_add_new_disk+0xe3b/0x1850 drivers/md/md.c:7684
   md_ioctl+0x200a/0x2610 drivers/md/md.c:8499

bind_rdev_to_array() is not the only way in.  md_run() calls
sysfs_create_group(&mddev->kobj, &md_redundancy_group), and
internal_create_group() has its own WARN_ON(!kobj->sd), so RUN_ARRAY in
the same window warns too.  Guarding the individual callers would mean
finding all of them; the window itself is what should not be reachable.

Refuse the open until md_alloc() has added the kobject.  mddev->kobj.sd
is NULL until kobject_add() creates the directory, and stays set for the
rest of the mddev's life: commit ca39f7502425 ("md: fix mddev->kobj
lifetime") dropped the explicit kobject_del() and lets the final put do
the removal.  md.c already tests mddev->kobj.sd for "is this array
published yet" in mddev_unlock() and md_run().

This cannot deadlock against add_disk() itself: device_add_disk() only
opens the disk to scan partitions when get_capacity(disk) is non-zero,
and md_alloc() does not set the capacity - md_run() does, long after.

Fixes: ca39f7502425 ("md: fix mddev->kobj lifetime")
Reported-by: syzbot+95eeb4ada2349a2170ea@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=95eeb4ada2349a2170ea
Cc: stable@vger.kernel.org
Assisted-by: LLM
---
 drivers/md/md.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 680b34a63cb3..e3326b78aef7 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -8610,6 +8610,15 @@ static int md_open(struct gendisk *disk, blk_mode_t mode)
 	if (test_bit(MD_CLOSING, &mddev->flags))
 		goto out_unlock;
 
+	/*
+	 * md_alloc() publishes the gendisk with add_disk() before it has
+	 * initialised and added mddev->kobj, so an array opened in that window
+	 * would let ioctls reach a kobject that is still all zeroes.  Wait for
+	 * md_alloc() to finish rather than handing out the device early.
+	 */
+	if (!mddev->kobj.sd)
+		goto out_unlock;
+
 	atomic_inc(&mddev->openers);
 	mutex_unlock(&mddev->open_mutex);
 
-- 
2.55.0.windows.5


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] md: don't hand out the array before md_alloc() has added mddev->kobj
  2026-10-04  5:57 [PATCH] md: don't hand out the array before md_alloc() has added mddev->kobj Yogesh Gaur
@ 2026-10-05 15:47 ` Logan Gunthorpe
  2026-10-05 17:05 ` yu kuai
  1 sibling, 0 replies; 4+ messages in thread
From: Logan Gunthorpe @ 2026-10-05 15:47 UTC (permalink / raw)
  To: Yogesh Gaur, Song Liu, Yu Kuai
  Cc: linux-raid, linux-kernel, Li Nan, Xiao Ni, Christoph Hellwig,
	Hannes Reinecke, syzbot+95eeb4ada2349a2170ea, stable



On 2026-10-03 23:57, Yogesh Gaur wrote:
> md_alloc() publishes the gendisk before it is done setting the mddev up:
> 
> 	disk->private_data = mddev;
> 	...
> 	error = add_disk(disk);
> 	if (error)
> 		goto out_put_disk;
> 
> 	kobject_init(&mddev->kobj, &md_ktype);
> 	error = kobject_add(&mddev->kobj, &disk_to_dev(disk)->kobj, "%s", "md");
> 
> add_disk() makes /dev/mdN openable, and md_open() only refuses the open
> when MD_CLOSING is set.  mddev comes from mddev_alloc() and is zeroed,
> so anything that gets in between add_disk() and kobject_add() sees an
> mddev->kobj that has never been through kobject_init() - no ktype, no
> kref, state_initialized clear.
> 
> syzbot opens the array in that window and issues ADD_NEW_DISK:
> 
>   kobject: '(null)' (ffff8880120640f0): is not initialized, yet kobject_get() is being called.
>   WARNING: lib/kobject.c:642 at kobject_add_internal+0xea/0xcd0 lib/kobject.c:225
>    kobject_add_varg lib/kobject.c:374 [inline]
>    kobject_add+0x163/0x240 lib/kobject.c:426
>    bind_rdev_to_array+0x80c/0xdd0 drivers/md/md.c:2621
>    md_add_new_disk+0xe3b/0x1850 drivers/md/md.c:7684
>    md_ioctl+0x200a/0x2610 drivers/md/md.c:8499
> 
> bind_rdev_to_array() is not the only way in.  md_run() calls
> sysfs_create_group(&mddev->kobj, &md_redundancy_group), and
> internal_create_group() has its own WARN_ON(!kobj->sd), so RUN_ARRAY in
> the same window warns too.  Guarding the individual callers would mean
> finding all of them; the window itself is what should not be reachable.
> 
> Refuse the open until md_alloc() has added the kobject.  mddev->kobj.sd
> is NULL until kobject_add() creates the directory, and stays set for the
> rest of the mddev's life: commit ca39f7502425 ("md: fix mddev->kobj
> lifetime") dropped the explicit kobject_del() and lets the final put do
> the removal.  md.c already tests mddev->kobj.sd for "is this array
> published yet" in mddev_unlock() and md_run().
> 
> This cannot deadlock against add_disk() itself: device_add_disk() only
> opens the disk to scan partitions when get_capacity(disk) is non-zero,
> and md_alloc() does not set the capacity - md_run() does, long after.
> 
> Fixes: ca39f7502425 ("md: fix mddev->kobj lifetime")
> Reported-by: syzbot+95eeb4ada2349a2170ea@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=95eeb4ada2349a2170ea
> Cc: stable@vger.kernel.org
> Assisted-by: LLM

This makes sense to me, thanks.

Reviewed by: Logan Gunthorpe <logang@deltatee.com>

Logan

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] md: don't hand out the array before md_alloc() has added mddev->kobj
  2026-10-04  5:57 [PATCH] md: don't hand out the array before md_alloc() has added mddev->kobj Yogesh Gaur
  2026-10-05 15:47 ` Logan Gunthorpe
@ 2026-10-05 17:05 ` yu kuai
  2026-10-06  3:43   ` Yogesh Gaur
  1 sibling, 1 reply; 4+ messages in thread
From: yu kuai @ 2026-10-05 17:05 UTC (permalink / raw)
  To: Yogesh Gaur, Song Liu, yu kuai
  Cc: linux-raid, linux-kernel, Li Nan, Xiao Ni, Christoph Hellwig,
	Hannes Reinecke, Logan Gunthorpe, syzbot+95eeb4ada2349a2170ea,
	stable

Hi,

在 2026/10/4 13:57, Yogesh Gaur 写道:
> md_alloc() publishes the gendisk before it is done setting the mddev up:
>
> 	disk->private_data = mddev;
> 	...
> 	error = add_disk(disk);
> 	if (error)
> 		goto out_put_disk;
>
> 	kobject_init(&mddev->kobj, &md_ktype);
> 	error = kobject_add(&mddev->kobj, &disk_to_dev(disk)->kobj, "%s", "md");
>
> add_disk() makes /dev/mdN openable, and md_open() only refuses the open
> when MD_CLOSING is set.  mddev comes from mddev_alloc() and is zeroed,
> so anything that gets in between add_disk() and kobject_add() sees an
> mddev->kobj that has never been through kobject_init() - no ktype, no
> kref, state_initialized clear.
>
> syzbot opens the array in that window and issues ADD_NEW_DISK:
>
>    kobject: '(null)' (ffff8880120640f0): is not initialized, yet kobject_get() is being called.
>    WARNING: lib/kobject.c:642 at kobject_add_internal+0xea/0xcd0 lib/kobject.c:225
>     kobject_add_varg lib/kobject.c:374 [inline]
>     kobject_add+0x163/0x240 lib/kobject.c:426
>     bind_rdev_to_array+0x80c/0xdd0 drivers/md/md.c:2621
>     md_add_new_disk+0xe3b/0x1850 drivers/md/md.c:7684
>     md_ioctl+0x200a/0x2610 drivers/md/md.c:8499
>
> bind_rdev_to_array() is not the only way in.  md_run() calls
> sysfs_create_group(&mddev->kobj, &md_redundancy_group), and
> internal_create_group() has its own WARN_ON(!kobj->sd), so RUN_ARRAY in
> the same window warns too.  Guarding the individual callers would mean
> finding all of them; the window itself is what should not be reachable.
>
> Refuse the open until md_alloc() has added the kobject.  mddev->kobj.sd
> is NULL until kobject_add() creates the directory, and stays set for the
> rest of the mddev's life: commit ca39f7502425 ("md: fix mddev->kobj
> lifetime") dropped the explicit kobject_del() and lets the final put do
> the removal.  md.c already tests mddev->kobj.sd for "is this array
> published yet" in mddev_unlock() and md_run().
>
> This cannot deadlock against add_disk() itself: device_add_disk() only
> opens the disk to scan partitions when get_capacity(disk) is non-zero,
> and md_alloc() does not set the capacity - md_run() does, long after.
>
> Fixes: ca39f7502425 ("md: fix mddev->kobj lifetime")
> Reported-by: syzbot+95eeb4ada2349a2170ea@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=95eeb4ada2349a2170ea
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> ---
>   drivers/md/md.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 680b34a63cb3..e3326b78aef7 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -8610,6 +8610,15 @@ static int md_open(struct gendisk *disk, blk_mode_t mode)
>   	if (test_bit(MD_CLOSING, &mddev->flags))
>   		goto out_unlock;
>   
> +	/*
> +	 * md_alloc() publishes the gendisk with add_disk() before it has
> +	 * initialised and added mddev->kobj, so an array opened in that window
> +	 * would let ioctls reach a kobject that is still all zeroes.  Wait for
> +	 * md_alloc() to finish rather than handing out the device early.
> +	 */
> +	if (!mddev->kobj.sd)
> +		goto out_unlock;
> +
>   	atomic_inc(&mddev->openers);
>   	mutex_unlock(&mddev->open_mutex);

This is the same problem that I already replied in another thread, the same solution
with different code.

Re: [PATCH] md: prevent opening an array before its sysfs kobject is 
ready - yu kuai <https://lore.kernel.org/all/a19e3020-272b-4f93-8535-00fa07002d91@fygo.io/#t>

>   

-- 
Thanks,
Kuai

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] md: don't hand out the array before md_alloc() has added mddev->kobj
  2026-10-05 17:05 ` yu kuai
@ 2026-10-06  3:43   ` Yogesh Gaur
  0 siblings, 0 replies; 4+ messages in thread
From: Yogesh Gaur @ 2026-10-06  3:43 UTC (permalink / raw)
  To: yukuai
  Cc: Song Liu, linux-raid, linux-kernel, Li Nan, Xiao Ni,
	Christoph Hellwig, Hannes Reinecke, Logan Gunthorpe,
	syzbot+95eeb4ada2349a2170ea, stable, xiaobai050

On Mon, Oct 5, 2026 at 10:35 PM yu kuai <yukuai@fygo.io> wrote:
>
> Hi,
>
> 在 2026/10/4 13:57, Yogesh Gaur 写道:
> > md_alloc() publishes the gendisk before it is done setting the mddev up:
> >
> >       disk->private_data = mddev;
> >       ...
> >       error = add_disk(disk);
> >       if (error)
> >               goto out_put_disk;
> >
> >       kobject_init(&mddev->kobj, &md_ktype);
> >       error = kobject_add(&mddev->kobj, &disk_to_dev(disk)->kobj, "%s", "md");
> >
> > add_disk() makes /dev/mdN openable, and md_open() only refuses the open
> > when MD_CLOSING is set.  mddev comes from mddev_alloc() and is zeroed,
> > so anything that gets in between add_disk() and kobject_add() sees an
> > mddev->kobj that has never been through kobject_init() - no ktype, no
> > kref, state_initialized clear.
> >
> > syzbot opens the array in that window and issues ADD_NEW_DISK:
> >
> >    kobject: '(null)' (ffff8880120640f0): is not initialized, yet kobject_get() is being called.
> >    WARNING: lib/kobject.c:642 at kobject_add_internal+0xea/0xcd0 lib/kobject.c:225
> >     kobject_add_varg lib/kobject.c:374 [inline]
> >     kobject_add+0x163/0x240 lib/kobject.c:426
> >     bind_rdev_to_array+0x80c/0xdd0 drivers/md/md.c:2621
> >     md_add_new_disk+0xe3b/0x1850 drivers/md/md.c:7684
> >     md_ioctl+0x200a/0x2610 drivers/md/md.c:8499
> >
> > bind_rdev_to_array() is not the only way in.  md_run() calls
> > sysfs_create_group(&mddev->kobj, &md_redundancy_group), and
> > internal_create_group() has its own WARN_ON(!kobj->sd), so RUN_ARRAY in
> > the same window warns too.  Guarding the individual callers would mean
> > finding all of them; the window itself is what should not be reachable.
> >
> > Refuse the open until md_alloc() has added the kobject.  mddev->kobj.sd
> > is NULL until kobject_add() creates the directory, and stays set for the
> > rest of the mddev's life: commit ca39f7502425 ("md: fix mddev->kobj
> > lifetime") dropped the explicit kobject_del() and lets the final put do
> > the removal.  md.c already tests mddev->kobj.sd for "is this array
> > published yet" in mddev_unlock() and md_run().
> >
> > This cannot deadlock against add_disk() itself: device_add_disk() only
> > opens the disk to scan partitions when get_capacity(disk) is non-zero,
> > and md_alloc() does not set the capacity - md_run() does, long after.
> >
> > Fixes: ca39f7502425 ("md: fix mddev->kobj lifetime")
> > Reported-by: syzbot+95eeb4ada2349a2170ea@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=95eeb4ada2349a2170ea
> > Cc: stable@vger.kernel.org
> > Assisted-by: LLM
> > ---
> >   drivers/md/md.c | 9 +++++++++
> >   1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/md/md.c b/drivers/md/md.c
> > index 680b34a63cb3..e3326b78aef7 100644
> > --- a/drivers/md/md.c
> > +++ b/drivers/md/md.c
> > @@ -8610,6 +8610,15 @@ static int md_open(struct gendisk *disk, blk_mode_t mode)
> >       if (test_bit(MD_CLOSING, &mddev->flags))
> >               goto out_unlock;
> >
> > +     /*
> > +      * md_alloc() publishes the gendisk with add_disk() before it has
> > +      * initialised and added mddev->kobj, so an array opened in that window
> > +      * would let ioctls reach a kobject that is still all zeroes.  Wait for
> > +      * md_alloc() to finish rather than handing out the device early.
> > +      */
> > +     if (!mddev->kobj.sd)
> > +             goto out_unlock;
> > +
> >       atomic_inc(&mddev->openers);
> >       mutex_unlock(&mddev->open_mutex);
>
> This is the same problem that I already replied in another thread, the same solution
> with different code.
>
> Re: [PATCH] md: prevent opening an array before its sysfs kobject is
> ready - yu kuai <https://lore.kernel.org/all/a19e3020-272b-4f93-8535-00fa07002d91@fygo.io/#t>
>

Thanks Kuai. Yes just refusing the open would be going to move the
race condition. I had missed earlier patch from xiaobai050, sorry for
the duplicate.

I would send a v2 that holds reconfig_mutex from add_disk() through
kobject_add() in md_alloc(), so an ioctl issued in that window waits
instead of failing the open. RUN_ARRAY is covered as well, since
md_run() also uses mddev->kobj and runs under same lock.

Thanks
Yogesh
> >
>
> --
> Thanks,
> Kuai

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-10-06  3:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-04  5:57 [PATCH] md: don't hand out the array before md_alloc() has added mddev->kobj Yogesh Gaur
2026-10-05 15:47 ` Logan Gunthorpe
2026-10-05 17:05 ` yu kuai
2026-10-06  3:43   ` Yogesh Gaur

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®