[patch] dm raid: pointer math issue in super_sync()

[Dan Carpenter]

"sb" is a dm_raid_superblock struct pointer so the pointer math doesn't
work and we will end up corrupting memory.

Signed-off-by: Dan Carpenter <***@oracle.com>

diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index b802644..a7cb9dd 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -826,7 +826,7 @@ static void super_sync(struct mddev *mddev, struct md_rdev *rdev)
test_bit(Faulty, &(rs->dev[i].rdev.flags)))
failed_devices |= (1ULL << i);

- memset(sb + sizeof(*sb), 0, rdev->sb_size - sizeof(*sb));
+ memset(sb + 1, 0, rdev->sb_size - sizeof(*sb));

sb->magic = cpu_to_le32(DM_RAID_MAGIC);
sb->features = cpu_to_le32(0); /* No features yet */
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Mike Snitzer]

On Tue, Oct 21 2014 at 8:43am -0400,

Post by Dan Carpenter
"sb" is a dm_raid_superblock struct pointer so the pointer math doesn't
work and we will end up corrupting memory.
diff --git a/drivers/md/dm-raid.c b/drivers/md/dm-raid.c
index b802644..a7cb9dd 100644
--- a/drivers/md/dm-raid.c
+++ b/drivers/md/dm-raid.c
@@ -826,7 +826,7 @@ static void super_sync(struct mddev *mddev, struct md_rdev *rdev)
test_bit(Faulty, &(rs->dev[i].rdev.flags)))
failed_devices |= (1ULL << i);
- memset(sb + sizeof(*sb), 0, rdev->sb_size - sizeof(*sb));
+ memset(sb + 1, 0, rdev->sb_size - sizeof(*sb));
sb->magic = cpu_to_le32(DM_RAID_MAGIC);
sb->features = cpu_to_le32(0); /* No features yet */

Not following... sizeof(*sb) != sizeof(sb). So I'm not seeing a
problem.

Nor am I seeing how you think sb + 1 is equivalent to what Heinz
intended (zero the memory following the sizeof(struct dm_raid_superblock)).
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Dan Carpenter]

Post by Mike Snitzer

Post by Dan Carpenter
- memset(sb + sizeof(*sb), 0, rdev->sb_size - sizeof(*sb));
+ memset(sb + 1, 0, rdev->sb_size - sizeof(*sb));
sb->magic = cpu_to_le32(DM_RAID_MAGIC);
sb->features = cpu_to_le32(0); /* No features yet */

Not following... sizeof(*sb) != sizeof(sb). So I'm not seeing a
problem.
Nor am I seeing how you think sb + 1 is equivalent to what Heinz
intended (zero the memory following the sizeof(struct dm_raid_superblock)).

It's pointer math.

sizeof(*sb) is 512.

"sb + sizeof(*sb)" is the same as (void *)sb + 512 * 512.
"sb + 1" is the same as (void *)sb + 512.

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Mike Snitzer]

On Tue, Oct 21 2014 at 8:57am -0400,

Post by Dan Carpenter

Post by Mike Snitzer

Post by Dan Carpenter
- memset(sb + sizeof(*sb), 0, rdev->sb_size - sizeof(*sb));
+ memset(sb + 1, 0, rdev->sb_size - sizeof(*sb));
sb->magic = cpu_to_le32(DM_RAID_MAGIC);
sb->features = cpu_to_le32(0); /* No features yet */

Not following... sizeof(*sb) != sizeof(sb). So I'm not seeing a
problem.
Nor am I seeing how you think sb + 1 is equivalent to what Heinz
intended (zero the memory following the sizeof(struct dm_raid_superblock)).

It's pointer math.

Yes, I see that now..

Post by Dan Carpenter
sizeof(*sb) is 512.
"sb + sizeof(*sb)" is the same as (void *)sb + 512 * 512.
"sb + 1" is the same as (void *)sb + 512.

Actually, Heinz removed the 452 bytes of padding from struct
dm_raid_superblock, so it is more like:

sizeof(*sb) == sizeof(struct dm_raid_super_block) == 60

"sb + sizeof(*sb)" is the same as (void *)sb + 60 * 60
"sb + 1" is the same as (void *)sb + 60.

But regardless, your broader point on the math stands. I'll get this
fixed up, thanks!

Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-raid" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html