[PATCH] md/raid5: protect lockless recovery_offset accesses during reshape
From: Chen Cheng <hidden>
Date: 2026-06-27 10:25:40
Also in:
lkml
Subsystem:
software raid (multiple disks) support, the rest · Maintainers:
Song Liu, Yu Kuai, Linus Torvalds
From: Chen Cheng <redacted>
During reshape:
- reshape_request() advances rdev->recovery_offset for non-In_sync
devices locklessly.
- analyse_stripe() reads rdev->recovery_offset locklessly to decide:
a. use a replacement device to read ?
b. a device can already be treated as in-sync for the current
stripe ?
one possible scenario is:
CPU1 CPU2
reshape_request()
-> mddev->curr_resync_completed = sector_nr
-> if (!mddev->reshape_backwards)
-> rdev->recovery_offset = sector_nr
analyse_stripe(sh)
-> rdev = conf->disks[i].replacement
-> if (rdev->recovery_offset >=
sh->sector + stripe_sectors)
set_bit(R5_ReadRepl)
-> or
-> if (sh->sector + stripe_sectors <=
rdev->recovery_offset)
set_bit(R5_Insync)
And it could be:
- reading from a replacement before it is recovered far enough; or
- treating a not-yet-recovered device as in-sync for the current stripe.
Fixes: db0505d32066 ("md: be cautious about using ->curr_resync_completed for ->recovery_offset")
The race report:
==================================================================
BUG: KCSAN: data-race in ops_run_io / reshape_request
write to 0xffff8bdee168b270 of 8 bytes by task 1704 on cpu 10:
reshape_request+0x1292/0x17b0
raid5_sync_request+0x815/0xa00
md_do_sync.cold+0xf8d/0x1516
[......]
read to 0xffff8bdee168b270 of 8 bytes by task 1696 on cpu 9:
ops_run_io+0xc25/0x1960
handle_stripe+0x2273/0x4570
handle_active_stripes.isra.0+0x6e0/0xa50
raid5d+0x7d5/0xb90
[......]
value changed: 0x0000000000091a00 -> 0x0000000000091b00
==================================================================
Signed-off-by: Chen Cheng <redacted>
---
drivers/md/raid5.c | 34 +++++++++++++++++-----------------
1 file changed, 17 insertions(+), 17 deletions(-)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index cacdf4211628..eaee7f206ab8 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c@@ -3808,11 +3808,11 @@ static int want_replace(struct stripe_head *sh, int disk_idx) rdev = sh->raid_conf->disks[disk_idx].replacement; if (rdev && !test_bit(Faulty, &rdev->flags) && !test_bit(In_sync, &rdev->flags) - && (rdev->recovery_offset <= sh->sector + && (READ_ONCE(rdev->recovery_offset) <= sh->sector || rdev->mddev->resync_offset <= sh->sector)) rv = 1; return rv; }
@@ -4726,11 +4726,11 @@ static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s) /* Prefer to use the replacement for reads, but only * if it is recovered enough and has no bad blocks. */ rdev = conf->disks[i].replacement; if (rdev && !test_bit(Faulty, &rdev->flags) && - rdev->recovery_offset >= sh->sector + RAID5_STRIPE_SECTORS(conf) && + READ_ONCE(rdev->recovery_offset) >= sh->sector + RAID5_STRIPE_SECTORS(conf) && !rdev_has_badblock(rdev, sh->sector, RAID5_STRIPE_SECTORS(conf))) set_bit(R5_ReadRepl, &dev->flags); else { if (rdev && !test_bit(Faulty, &rdev->flags))
@@ -4768,11 +4768,11 @@ static void analyse_stripe(struct stripe_head *sh, struct stripe_head_state *s) set_bit(R5_ReadError, &dev->flags); } } else if (test_bit(In_sync, &rdev->flags)) set_bit(R5_Insync, &dev->flags); else if (sh->sector + RAID5_STRIPE_SECTORS(conf) <= - rdev->recovery_offset) { + READ_ONCE(rdev->recovery_offset)) { /* * in sync if: * - normal IO, or * - resync IO that is not lazy recovery *
@@ -5514,17 +5514,17 @@ static int raid5_read_one_chunk(struct mddev *mddev, struct bio *raid_bio) if (r5c_big_stripe_cached(conf, sector)) return 0; rdev = conf->disks[dd_idx].replacement; if (!rdev || test_bit(Faulty, &rdev->flags) || - rdev->recovery_offset < end_sector) { + READ_ONCE(rdev->recovery_offset) < end_sector) { rdev = conf->disks[dd_idx].rdev; if (!rdev) return 0; if (test_bit(Faulty, &rdev->flags) || !(test_bit(In_sync, &rdev->flags) || - rdev->recovery_offset >= end_sector)) + READ_ONCE(rdev->recovery_offset) >= end_sector)) return 0; } atomic_inc(&rdev->nr_pending);
@@ -6463,12 +6463,12 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk /* Can update recovery_offset */ rdev_for_each(rdev, mddev) if (rdev->raid_disk >= 0 && !test_bit(Journal, &rdev->flags) && !test_bit(In_sync, &rdev->flags) && - rdev->recovery_offset < sector_nr) - rdev->recovery_offset = sector_nr; + READ_ONCE(rdev->recovery_offset) < sector_nr) + WRITE_ONCE(rdev->recovery_offset, sector_nr); conf->reshape_checkpoint = jiffies; set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags); md_wakeup_thread(mddev->thread); wait_event(mddev->sb_wait, READ_ONCE(mddev->sb_flags) == 0 ||
@@ -6572,12 +6572,12 @@ static sector_t reshape_request(struct mddev *mddev, sector_t sector_nr, int *sk /* Can update recovery_offset */ rdev_for_each(rdev, mddev) if (rdev->raid_disk >= 0 && !test_bit(Journal, &rdev->flags) && !test_bit(In_sync, &rdev->flags) && - rdev->recovery_offset < sector_nr) - rdev->recovery_offset = sector_nr; + READ_ONCE(rdev->recovery_offset) < sector_nr) + WRITE_ONCE(rdev->recovery_offset, sector_nr); conf->reshape_checkpoint = jiffies; set_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags); md_wakeup_thread(mddev->thread); wait_event(mddev->sb_wait, !test_bit(MD_SB_CHANGE_DEVS, &mddev->sb_flags)
@@ -8106,13 +8106,13 @@ static int raid5_run(struct mddev *mddev) * to worry about reshape going forwards. */ /* Hack because v0.91 doesn't store recovery_offset properly. */ if (mddev->major_version == 0 && mddev->minor_version > 90) - rdev->recovery_offset = reshape_offset; + WRITE_ONCE(rdev->recovery_offset, reshape_offset); - if (rdev->recovery_offset < reshape_offset) { + if (READ_ONCE(rdev->recovery_offset) < reshape_offset) { /* We need to check old and new layout */ if (!only_parity(rdev->raid_disk, conf->algorithm, conf->raid_disks, conf->max_degraded))
@@ -8264,11 +8264,11 @@ static int raid5_spare_active(struct mddev *mddev) for (i = 0; i < conf->raid_disks; i++) { rdev = conf->disks[i].rdev; replacement = conf->disks[i].replacement; if (replacement - && replacement->recovery_offset == MaxSector + && READ_ONCE(replacement->recovery_offset) == MaxSector && !test_bit(Faulty, &replacement->flags) && !test_and_set_bit(In_sync, &replacement->flags)) { /* Replacement has just become active. */ if (!rdev || !test_and_clear_bit(In_sync, &rdev->flags))
@@ -8282,13 +8282,13 @@ static int raid5_spare_active(struct mddev *mddev) sysfs_notify_dirent_safe( rdev->sysfs_state); } sysfs_notify_dirent_safe(replacement->sysfs_state); } else if (rdev - && rdev->recovery_offset == MaxSector - && !test_bit(Faulty, &rdev->flags) - && !test_and_set_bit(In_sync, &rdev->flags)) { + && READ_ONCE(rdev->recovery_offset) == MaxSector + && !test_bit(Faulty, &rdev->flags) + && !test_and_set_bit(In_sync, &rdev->flags)) { count++; sysfs_notify_dirent_safe(rdev->sysfs_state); } } spin_lock_irqsave(&conf->device_lock, flags);
@@ -8653,11 +8653,11 @@ static int raid5_start_reshape(struct mddev *mddev) if (raid5_add_disk(mddev, rdev) == 0) { if (rdev->raid_disk >= conf->previous_raid_disks) set_bit(In_sync, &rdev->flags); else - rdev->recovery_offset = 0; + WRITE_ONCE(rdev->recovery_offset, 0); /* Failure here is OK */ sysfs_link_rdev(mddev, rdev); } } else if (rdev->raid_disk >= conf->previous_raid_disks
@@ -8705,11 +8705,11 @@ static void end_reshape(struct r5conf *conf) conf->mddev->reshape_position = MaxSector; rdev_for_each(rdev, conf->mddev) if (rdev->raid_disk >= 0 && !test_bit(Journal, &rdev->flags) && !test_bit(In_sync, &rdev->flags)) - rdev->recovery_offset = MaxSector; + WRITE_ONCE(rdev->recovery_offset, MaxSector); spin_unlock_irq(&conf->device_lock); wake_up(&conf->wait_for_reshape); mddev_update_io_opt(conf->mddev, conf->raid_disks - conf->max_degraded);
--
2.54.0