linux-kernel - Re: [PATCH v9 4/5] md: add check_new

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a660478f-b146-05ec-a3f4-f86457b096d0@huaweicloud.com>
Date: Tue, 4 Nov 2025 10:52:06 +0800
From: Li Nan <linan666@...weicloud.com>
To: Xiao Ni <xni@...hat.com>, linan666@...weicloud.com
Cc: corbet@....net, song@...nel.org, yukuai@...as.com, hare@...e.de,
 linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-raid@...r.kernel.org, yangerkun@...wei.com, yi.zhang@...wei.com
Subject: Re: [PATCH v9 4/5] md: add check_new_feature module parameter



在 2025/11/4 9:47, Xiao Ni 写道:
> On Mon, Nov 3, 2025 at 9:06 PM <linan666@...weicloud.com> wrote:
>>
>> From: Li Nan <linan122@...wei.com>
>>
>> Raid checks if pad3 is zero when loading superblock from disk. Arrays
>> created with new features may fail to assemble on old kernels as pad3
>> is used.
>>
>> Add module parameter check_new_feature to bypass this check.
>>
>> Signed-off-by: Li Nan <linan122@...wei.com>
>> ---
>>   drivers/md/md.c | 12 +++++++++---
>>   1 file changed, 9 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index dffc6a482181..5921fb245bfa 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -339,6 +339,7 @@ static int start_readonly;
>>    */
>>   static bool create_on_open = true;
>>   static bool legacy_async_del_gendisk = true;
>> +static bool check_new_feature = true;
>>
>>   /*
>>    * We have a system wide 'event count' that is incremented
>> @@ -1850,9 +1851,13 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
>>          }
>>          if (sb->pad0 ||
>>              sb->pad3[0] ||
>> -           memcmp(sb->pad3, sb->pad3+1, sizeof(sb->pad3) - sizeof(sb->pad3[1])))
>> -               /* Some padding is non-zero, might be a new feature */
>> -               return -EINVAL;
>> +           memcmp(sb->pad3, sb->pad3+1, sizeof(sb->pad3) - sizeof(sb->pad3[1]))) {
>> +               pr_warn("Some padding is non-zero on %pg, might be a new feature\n",
>> +                       rdev->bdev);
>> +               if (check_new_feature)
>> +                       return -EINVAL;
>> +               pr_warn("check_new_feature is disabled, data corruption possible\n");
>> +       }
>>
>>          rdev->preferred_minor = 0xffff;
>>          rdev->data_offset = le64_to_cpu(sb->data_offset);
>> @@ -10704,6 +10709,7 @@ module_param(start_dirty_degraded, int, S_IRUGO|S_IWUSR);
>>   module_param_call(new_array, add_named_array, NULL, NULL, S_IWUSR);
>>   module_param(create_on_open, bool, S_IRUSR|S_IWUSR);
>>   module_param(legacy_async_del_gendisk, bool, 0600);
>> +module_param(check_new_feature, bool, 0600);
>>
>>   MODULE_LICENSE("GPL");
>>   MODULE_DESCRIPTION("MD RAID framework");
>> --
>> 2.39.2
>>
> 
> Hi
> 
> Thanks for finding this problem in time. The default of this kernel
> module is true. I don't think people can check new kernel modules
> after updating to a new kernel. They will find the array can't
> assemble and report bugs. You already use pad3, is it good to remove
> the check about pad3 directly here?
> 
> By the way, have you run the regression tests?
> 
> Regards
> Xiao
> 
> 
> .

Hi Xiao.

Thanks for your review.

Deleting this check directly is risky. For example, in configurable LBS:
if user sets LBS to 4K, the LBS of a RAID array assembled on old kernel
becomes 512. Forcing use of this array then risks data loss -- the
original issue this feature want to solve.

Future features may also have similar risks, so instead of deleting this
check directly, I chose to add a module parameter to give users a choice.
What do you think?

-- 
Thanks,
Nan