Skip to content

Commit

Permalink
dm raid: fix accesses beyond end of raid member array
Browse files Browse the repository at this point in the history
On dm-raid table load (using raid_ctr), dm-raid allocates an array
rs->devs[rs->raid_disks] for the raid device members. rs->raid_disks
is defined by the number of raid metadata and image tupples passed
into the target's constructor.

In the case of RAID layout changes being requested, that number can be
different from the current number of members for existing raid sets as
defined in their superblocks. Example RAID layout changes include:
- raid1 legs being added/removed
- raid4/5/6/10 number of stripes changed (stripe reshaping)
- takeover to higher raid level (e.g. raid5 -> raid6)

When accessing array members, rs->raid_disks must be used in control
loops instead of the potentially larger value in rs->md.raid_disks.
Otherwise it will cause memory access beyond the end of the rs->devs
array.

Fix this by changing code that is prone to out-of-bounds access.
Also fix validate_raid_redundancy() to validate all devices that are
added. Also, use braces to help clean up raid_iterate_devices().

The out-of-bounds memory accesses was discovered using KASAN.

This commit was verified to pass all LVM2 RAID tests (with KASAN
enabled).

Cc: stable@vger.kernel.org
Signed-off-by: Heinz Mauelshagen <heinzm@redhat.com>
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
  • Loading branch information
Heinz Mauelshagen authored and Mike Snitzer committed Jun 27, 2022
1 parent 90736eb commit 332bd07
Showing 1 changed file with 18 additions and 16 deletions.
34 changes: 18 additions & 16 deletions drivers/md/dm-raid.c
Original file line number Diff line number Diff line change
Expand Up @@ -1001,12 +1001,13 @@ static int validate_region_size(struct raid_set *rs, unsigned long region_size)
static int validate_raid_redundancy(struct raid_set *rs)
{
unsigned int i, rebuild_cnt = 0;
unsigned int rebuilds_per_group = 0, copies;
unsigned int rebuilds_per_group = 0, copies, raid_disks;
unsigned int group_size, last_group_start;

for (i = 0; i < rs->md.raid_disks; i++)
if (!test_bit(In_sync, &rs->dev[i].rdev.flags) ||
!rs->dev[i].rdev.sb_page)
for (i = 0; i < rs->raid_disks; i++)
if (!test_bit(FirstUse, &rs->dev[i].rdev.flags) &&
((!test_bit(In_sync, &rs->dev[i].rdev.flags) ||
!rs->dev[i].rdev.sb_page)))
rebuild_cnt++;

switch (rs->md.level) {
Expand Down Expand Up @@ -1046,8 +1047,9 @@ static int validate_raid_redundancy(struct raid_set *rs)
* A A B B C
* C D D E E
*/
raid_disks = min(rs->raid_disks, rs->md.raid_disks);
if (__is_raid10_near(rs->md.new_layout)) {
for (i = 0; i < rs->md.raid_disks; i++) {
for (i = 0; i < raid_disks; i++) {
if (!(i % copies))
rebuilds_per_group = 0;
if ((!rs->dev[i].rdev.sb_page ||
Expand All @@ -1070,10 +1072,10 @@ static int validate_raid_redundancy(struct raid_set *rs)
* results in the need to treat the last (potentially larger)
* set differently.
*/
group_size = (rs->md.raid_disks / copies);
last_group_start = (rs->md.raid_disks / group_size) - 1;
group_size = (raid_disks / copies);
last_group_start = (raid_disks / group_size) - 1;
last_group_start *= group_size;
for (i = 0; i < rs->md.raid_disks; i++) {
for (i = 0; i < raid_disks; i++) {
if (!(i % copies) && !(i > last_group_start))
rebuilds_per_group = 0;
if ((!rs->dev[i].rdev.sb_page ||
Expand Down Expand Up @@ -1588,7 +1590,7 @@ static sector_t __rdev_sectors(struct raid_set *rs)
{
int i;

for (i = 0; i < rs->md.raid_disks; i++) {
for (i = 0; i < rs->raid_disks; i++) {
struct md_rdev *rdev = &rs->dev[i].rdev;

if (!test_bit(Journal, &rdev->flags) &&
Expand Down Expand Up @@ -3766,13 +3768,13 @@ static int raid_iterate_devices(struct dm_target *ti,
unsigned int i;
int r = 0;

for (i = 0; !r && i < rs->md.raid_disks; i++)
if (rs->dev[i].data_dev)
r = fn(ti,
rs->dev[i].data_dev,
0, /* No offset on data devs */
rs->md.dev_sectors,
data);
for (i = 0; !r && i < rs->raid_disks; i++) {
if (rs->dev[i].data_dev) {
r = fn(ti, rs->dev[i].data_dev,
0, /* No offset on data devs */
rs->md.dev_sectors, data);
}
}

return r;
}
Expand Down

0 comments on commit 332bd07

Please sign in to comment.