[PATCH] dm-raid: check for zero feature flags in metadata

[PATCH] dm-raid: check for zero feature flags in metadata

[heinzm]

From: Heinz Mauelshagen <heinzm@redhat.com>

Because we don't support any feature flags in the dm-raid ondisk
metadata yet (see struct dm_raid_superblock, 'features' member), 
this patch adds a check to ensure no flags are actually set or
reject the activation of the RAID mapping.

This is to prevent possible data corruption in case of a
kernel downgrade when there'll potentially be feature flags
set by a future dm-raid target.


Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>

---
 drivers/md/dm-raid.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index a090121..5f05ba0 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -1037,6 +1037,10 @@ static int super_validate(struct raid_set *rs, struct md_rdev *rdev)
 	if (!mddev->events && super_init_validation(mddev, rdev))
 		return -EINVAL;
 
+	rs->ti->error = "Unable to assemble array: No feature flags supported yet";
+	if (le32_to_cpu(sb->features)) /* No features supported yet */
+		return -EINVAL;
+
 	/* Enable bitmap creation for RAID levels != 0 */
 	mddev->bitmap_info.offset = (rs->raid_type->level) ? to_sector(4096) : 0;
 	rdev->mddev->bitmap_info.default_offset = mddev->bitmap_info.offset;
-- 
2.5.5

Re: dm-raid: check for zero feature flags in metadata

[Mike Snitzer]

On Fri, Apr 29 2016 at 12:59pm -0400,
heinzm@redhat.com <heinzm@redhat.com> wrote:

> From: Heinz Mauelshagen <heinzm@redhat.com>
> 
> Because we don't support any feature flags in the dm-raid ondisk
> metadata yet (see struct dm_raid_superblock, 'features' member), 
> this patch adds a check to ensure no flags are actually set or
> reject the activation of the RAID mapping.
> 
> This is to prevent possible data corruption in case of a
> kernel downgrade when there'll potentially be feature flags
> set by a future dm-raid target.
> 
> 
> Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>

merged for 4.7, see:
https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.7&id=480375cd7ea3eed60381539dbfa96851b6174820

Re: dm-raid: check for zero feature flags in metadata

[Mike Snitzer]

On Fri, Apr 29 2016 at  3:18pm -0400,
Mike Snitzer <snitzer@redhat.com> wrote:

> On Fri, Apr 29 2016 at 12:59pm -0400,
> heinzm@redhat.com <heinzm@redhat.com> wrote:
> 
> > From: Heinz Mauelshagen <heinzm@redhat.com>
> > 
> > Because we don't support any feature flags in the dm-raid ondisk
> > metadata yet (see struct dm_raid_superblock, 'features' member), 
> > this patch adds a check to ensure no flags are actually set or
> > reject the activation of the RAID mapping.
> > 
> > This is to prevent possible data corruption in case of a
> > kernel downgrade when there'll potentially be feature flags
> > set by a future dm-raid target.
> > 
> > 
> > Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
> 
> merged for 4.7, see:
> https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.7&id=480375cd7ea3eed60381539dbfa96851b6174820

Actually, I revised the header and patch content, see:

https://git.kernel.org/cgit/linux/kernel/git/device-mapper/linux-dm.git/commit/?h=dm-4.7&id=ecdb57d205b98b656ab3ea0412f7a15ec003357f

Re: [PATCH] dm-raid: check for zero feature flags in metadata

[Alasdair G Kergon]

On Fri, Apr 29, 2016 at 06:59:56PM +0200, heinzm@redhat.com wrote:
> +	rs->ti->error = "Unable to assemble array: No feature flags supported yet";
> +	if (le32_to_cpu(sb->features)) /* No features supported yet */

Is it worth splitting this into compat_flags and incompat_flags like we did in
thin and cache? 

Alasdair

Re: [PATCH] dm-raid: check for zero feature flags in metadata

[Heinz Mauelshagen]

On 04/30/2016 02:31 AM, Alasdair G Kergon wrote:
> On Fri, Apr 29, 2016 at 06:59:56PM +0200, heinzm@redhat.com wrote:
>> +	rs->ti->error = "Unable to assemble array: No feature flags supported yet";
>> +	if (le32_to_cpu(sb->features)) /* No features supported yet */
> Is it worth splitting this into compat_flags and incompat_flags like we did in
> thin and cache?

I'm envisioning a single feature flag for future reshape/takeover 
functionality.
Seems like a good idea still so I'll add this to the RAID superblock 
metadata changes.

Heinz

>
> Alasdair
>
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel