[PATCH 0/2] md/raid5: fix PPL disk add and replacement removal

[PATCH 0/2] md/raid5: fix PPL disk add and replacement removal

[Yingjie Wang]

This series fixes two independent PPL disk-transition bugs.

When a newly added disk fails PPL setup, raid5_add_disk() can publish
the private disk pointer before the caller rolls back raid_disk. A later
removal can then free the disk while RAID5 still references it. Patch 1
prepares the PPL log before publishing the pointer and restores the
previous state on failure.

Patch 2 keeps the active PPL log attached when an unpromoted replacement
is removed. Without it, writes to the slot take the -EAGAIN path in
ppl_write_stripe() and are dispatched without PPL protection.

These fixes are independent of the separate replacement-activation work
and can be applied on their own.

Tested on x86_64 at the final commit:

  * per-commit W=1 KASAN+PROVE_LOCKING bzImage builds;
  * deterministic add rollback with -ENOSPC; and
  * removal of an unpromoted replacement, with entry/return kprobes on
    ppl_write_stripe().

On the unpatched md-7.4 base, the add-rollback test triggers a KASAN
slab-use-after-free in handle_stripe(), and the replacement-removal test
observes only -EAGAIN returns from ppl_write_stripe(). The final series
completes the add-rollback test and records a successful PPL write after
replacement removal. No KASAN, BUG, WARNING, Oops, or lockdep report was
observed on the fixed kernel.

All runtime tests used metadata=1.2. Empty-header -EIO injection and an
IMSM external-metadata runtime test were not run.

Yingjie Wang (2):
  md/raid5: roll back failed PPL disk additions
  md/raid5: preserve active PPL log when removing a replacement

 drivers/md/raid5-ppl.c | 11 ++++++++++-
 drivers/md/raid5.c     | 13 +++++++++----
 2 files changed, 19 insertions(+), 5 deletions(-)


base-commit: 47e1b18f38d74015cb9fd30b5ff2a1eb88e77d31
-- 
2.43.0

[PATCH 1/2] md/raid5: roll back failed PPL disk additions

[Yingjie Wang]

raid5_add_disk() publishes a new device in conf->disks[] before
preparing its PPL log. If PPL validation or the empty-header write
fails, the caller resets rdev->raid_disk, but the RAID5 private pointer
remains. A later removal can free the device while RAID5 still
references it.

Prepare the PPL log before publishing the RAID5 disk pointer and reset
rdev->raid_disk to -1 when preparation fails. If empty-header setup
fails after the PPL log pointer is published, restore the previous log
pointer and PPL size and do not initialize the child log.

Fixes: 6358c239d88c ("raid5-ppl: support disk hot add/remove with PPL")
Cc: stable@vger.kernel.org
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 drivers/md/raid5-ppl.c | 11 ++++++++++-
 drivers/md/raid5.c     |  9 +++++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/md/raid5-ppl.c b/drivers/md/raid5-ppl.c
index 0530632f9d2a..2ce4e0e72282 100644
--- a/drivers/md/raid5-ppl.c
+++ b/drivers/md/raid5-ppl.c
@@ -1461,6 +1461,8 @@ int ppl_modify_log(struct r5conf *conf, struct md_rdev *rdev, bool add)
 {
 	struct ppl_conf *ppl_conf = conf->log_private;
 	struct ppl_log *log;
+	struct md_rdev *old_rdev;
+	unsigned int old_ppl_size;
 	int ret = 0;
 
 	if (!rdev)
@@ -1480,11 +1482,18 @@ int ppl_modify_log(struct r5conf *conf, struct md_rdev *rdev, bool add)
 
 	mutex_lock(&log->io_mutex);
 	if (add) {
+		old_rdev = log->rdev;
+		old_ppl_size = rdev->ppl.size;
 		ret = ppl_validate_rdev(rdev);
 		if (!ret) {
 			log->rdev = rdev;
 			ret = ppl_write_empty_header(log);
-			ppl_init_child_log(log, rdev);
+			if (ret) {
+				log->rdev = old_rdev;
+				rdev->ppl.size = old_ppl_size;
+			} else {
+				ppl_init_child_log(log, rdev);
+			}
 		}
 	} else {
 		log->rdev = NULL;
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index efb4b0d37cb0..455e36ddceee 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -8550,12 +8550,17 @@ static int raid5_add_disk(struct mddev *mddev, struct md_rdev *rdev)
 		if (p->rdev == NULL) {
 			clear_bit(In_sync, &rdev->flags);
 			rdev->raid_disk = disk;
+
+			err = log_modify(conf, rdev, true);
+			if (err) {
+				rdev->raid_disk = -1;
+				goto out;
+			}
+
 			if (rdev->saved_raid_disk != disk)
 				conf->fullsync = 1;
 			WRITE_ONCE(p->rdev, rdev);
 
-			err = log_modify(conf, rdev, true);
-
 			goto out;
 		}
 	}
-- 
2.43.0

[PATCH 2/2] md/raid5: preserve active PPL log when removing a replacement

[Yingjie Wang]

raid5_remove_disk() selects either the active member or its replacement,
but then clears the slot PPL log regardless of which device is being
removed. Removing an unpromoted replacement therefore sets log->rdev to
NULL even though the active member remains in the slot.

ppl_write_stripe() subsequently returns -EAGAIN for that slot, which
lets ops_run_io() dispatch the writes without first recording them in
PPL.

Only change the PPL log when the active rdev is removed. Keep it attached
to the active member while an unpromoted replacement is detached.

Fixes: 6358c239d88c ("raid5-ppl: support disk hot add/remove with PPL")
Cc: stable@vger.kernel.org
Signed-off-by: Yingjie Wang <x-professor@outlook.com>
---
 drivers/md/raid5.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index 455e36ddceee..2f4fe416113c 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -8474,12 +8474,12 @@ static int raid5_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
 		err = -EBUSY;
 		goto abort;
 	}
-	WRITE_ONCE(*rdevp, NULL);
-	if (!err) {
+	if (rdev == p->rdev) {
 		err = log_modify(conf, rdev, false);
 		if (err)
 			goto abort;
 	}
+	WRITE_ONCE(*rdevp, NULL);
 
 	tmp = p->replacement;
 	if (tmp) {
-- 
2.43.0

Re: [PATCH 2/2] md/raid5: preserve active PPL log when removing a replacement

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] md/raid5: raid5_remove_disk() fails to clear slot beyond
  raid_disks with PPL
--

commit ade16f0f32de5eb72fa3bf67b1040d7514f41d37
Author: Yingjie Wang <x-professor@outlook.com>

md/raid5: preserve active PPL log when removing a replacement

This commit restricts the log_modify(conf, rdev, false) call in
raid5_remove_disk() to cases where the removed device is the slot's active
rdev rather than a replacement device, and moves clearing *rdevp after
log_modify().

> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 455e36ddceeed..2f4fe416113c0 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -8474,12 +8474,12 @@ static int raid5_remove_disk(struct mddev *mddev, struct md_rdev *rdev)
>  		err = -EBUSY;
>  		goto abort;
>  	}
> -	WRITE_ONCE(*rdevp, NULL);
> -	if (!err) {
> +	if (rdev == p->rdev) {
>  		err = log_modify(conf, rdev, false);
>  		if (err)
>  			goto abort;
>  	}
> +	WRITE_ONCE(*rdevp, NULL);

[Severity: Medium]
Does moving WRITE_ONCE(*rdevp, NULL) after log_modify() prevent removing a
disk in a slot where number >= conf->raid_disks when PPL is enabled?

After shrinking an array, conf->raid_disks is smaller than conf->pool_size
while extra disks remain in conf->disks[number].rdev.  If PPL is enabled
before those disks are detached, ppl_conf->count is initialized to
conf->raid_disks, and ppl_modify_log() rejects any slot at or beyond
conf->raid_disks:

drivers/md/raid5-ppl.c:ppl_modify_log() {
	...
	if (rdev->raid_disk >= ppl_conf->count)
		return -ENODEV;
	...
}

Jumping to abort on that error before clearing *rdevp leaves rdev stuck in
p->rdev, and every subsequent removal attempt fails with -ENODEV.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1794215128.git.x-professor@outlook.com?part=2