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

[Zizhi Wo <wozizhi@huaweicloud.com>]

Hi，yukuai

在 2026/11/6 14:34, yu kuai 写道:
> Hi,
> 
> 在 2026/11/2 16:38, Zizhi Wo 写道:
>> From: Zizhi Wo <wozizhi@huawei.com>
>>
>> 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@huawei.com>
>> ---
>>    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;
>