Re: [PATCH V3 1/3] md/raid0: reject a takeover whose zones cannot cover the array

[Zizhi Wo]

Hi，yukuai

在 2026/11/6 14:34, yu kuai 写道:
> Hi,
>
> 在 2026/11/2 16:38, Zizhi Wo 写道:
> > From: Zizhi Wo <wozizhi@xxxxxxxxxx>
> >
> > If a raid0 takeover builds strip zones that cover less than the advertised
> > array size, the block layer still accepts I/O for the whole size.
> > raid0_make_request() hands any request past the zones to the mapping code,
> > which fails to map it and triggers a BUG_ON.
> >
> > The zones are built from the member sizes in rdev->sectors, which may not
> > match the size the array already advertises, so the zones can come out
> > covering less than it. The extreme case is a member with no size recorded
> > at all: an array created without metadata imports its devices through
> > md_import_device(dev, -1, -1), which never sets rdev->sectors, and
> > bind_rdev_to_array() does not reject such a member either, because its
> > check that mddev->dev_sectors fits inside a member is skipped when
> > rdev->sectors is zero.
> >
> > Refuse a raid0 takeover when its zones cannot cover the size the gendisk
> > advertises. This prevents the takeover from silently shrinking the array,
> > mirroring the rule raid5_start_reshape() and raid10_start_reshape() apply
> > to a reshape.
> >
> > The check iterates the same devices and uses the same rounding as
> > create_strip_zones(). It does not skip devices with rdev->raid_disk < 0:
> > such devices cannot be raid0 members, so counting them only overestimates
> > the coverage and, if present, the takeover fails later in
> > create_strip_zones() anyway.
> >
> > Fixes: 9af204cf720c ("md: Add support for Raid5->Raid0 and Raid10->Raid0 takeover")
> > Fixes: fc3a08b85b7a ("Add raid1->raid0 takeover support")
> > Signed-off-by: Zizhi Wo <wozizhi@xxxxxxxxxx>
> > ---
> >    drivers/md/raid0.c | 37 +++++++++++++++++++++++++++++++++++++
> >    1 file changed, 37 insertions(+)
> >
> > diff --git a/drivers/md/raid0.c b/drivers/md/raid0.c
> > index 35e103f0c2c3..fccc62a5fecc 100644
> > --- a/drivers/md/raid0.c
> > +++ b/drivers/md/raid0.c
> > @@ -643,6 +643,34 @@ static void raid0_error(struct mddev *mddev, struct md_rdev *rdev)
> >    	}
> >    }
> >    
> > +/*
> > + * Refuse a takeover whose strip zones cannot cover the capacity the gendisk
> > + * advertises: raid0 derives those zones from the sizes of its members, so
> > + * find_zone() cannot map I/O that falls beyond them.
> > + */
> > +static int raid0_takeover_check(struct mddev *mddev, int chunk)
> > +{
> > +	sector_t capacity = get_capacity(mddev->gendisk);
> > +	sector_t coverage = 0;
> > +	struct md_rdev *rdev;
> > +
> > +	rdev_for_each(rdev, mddev) {
> > +		sector_t sectors = rdev->sectors;
> > +
> > +		sector_div(sectors, chunk);
> > +		coverage += sectors * chunk;
> > +	}
> > +
> > +	if (coverage < capacity) {
> > +		pr_warn("md/raid0:%s: refusing takeover, zones cover only %llu of %llu sectors\n",
> > +			mdname(mddev), (unsigned long long)coverage,
> > +			(unsigned long long)capacity);
>
> Other than raid1, other levels already have mddev->chunk_secotrs set, and raid456/raid10 already
> have a full functional raid0, their takeover can not fail.
>
> Noted normally mdadm will set rdev->sectors and 1.x metadata will reject zero rdev->sectors in super_1_load.
> However, it's still possible raid1 can be created with rdev->sectors be zero, this is the root cause of the
> problem syzbot reported.

Please consider the following two cases, both of which reproduce the
same BUG_ON locally for me:

1. An array created without metadata leaves rdev->sectors at 0. This
isn't raid1-specific — raid10 hits the same BUG_ON too. raid45 only
escapes because raid0_takeover_raid45() overwrites rdev->sectors with
mddev->dev_sectors first; raid0_takeover_raid10() doesn't.

2. Even with valid non-zero rdev->sectors, growing the chunk of a
running raid45 can make the post-takeover coverage smaller than the
previously advertised gendisk capacity. E.g. create a raid5 with
component_size=8K, chunk=4K, run it, then change chunk to 16K and run
raid0_takeover_raid45().

So I think the real root cause is that the size changes across the
takeover, and rdev->sectors == 0 is just one special case of it. That's
why the patch compares coverage against the gendisk capacity on every
takeover path.

For case 2, the problem exists even without a takeover: if the chunk
change shrinks the usable size, I/O from the upper filesystem will break
just the same. So I'm now inclined to reject a chunk change in raid5/10
when it would shrink the usable array size, and additionally check rdev-
>sectors for raid1/10 at the takeover path. What do you think?

Thanks,
Zizhi Wo

> Although raid0_takeover_raid1() already calculate chunksect by mddev->array_sectors, which is exactly the
> real rdev->sectors for raid1, and it should be guaranteed such takeover can succeed. However, as it's legacy
> usage that rdev->sectors can be zero, I'll suggest just checking rdev->sectors in raid0_takeover_raid1() and
> reject if it's 0.
>
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >    static void *raid0_takeover_raid45(struct mddev *mddev)
> >    {
> >    	struct md_rdev *rdev;
> > @@ -665,6 +693,9 @@ static void *raid0_takeover_raid45(struct mddev *mddev)
> >    		rdev->sectors = mddev->dev_sectors;
> >    	}
> >    
> > +	if (raid0_takeover_check(mddev, mddev->chunk_sectors))
> > +		return ERR_PTR(-EINVAL);
> > +
> >    	/* Set new parameters */
> >    	mddev->new_level = 0;
> >    	mddev->new_layout = 0;
> > @@ -707,6 +738,9 @@ static void *raid0_takeover_raid10(struct mddev *mddev)
> >    		return ERR_PTR(-EINVAL);
> >    	}
> >    
> > +	if (raid0_takeover_check(mddev, mddev->chunk_sectors))
> > +		return ERR_PTR(-EINVAL);
> > +
> >    	/* Set new parameters */
> >    	mddev->new_level = 0;
> >    	mddev->new_layout = 0;
> > @@ -750,6 +784,9 @@ static void *raid0_takeover_raid1(struct mddev *mddev)
> >    		/* array size does not allow a suitable chunk size */
> >    		return ERR_PTR(-EINVAL);
> >    
> > +	if (raid0_takeover_check(mddev, chunksect))
> > +		return ERR_PTR(-EINVAL);
> > +
> >    	/* Set new parameters */
> >    	mddev->new_level = 0;
> >    	mddev->new_layout = 0;