[PATCH] md/raid1: create serial pool adding rdev to array with serialize_policy=1

[PATCH] md/raid1: create serial pool adding rdev to array with serialize_policy=1

[Martin Wilck]

The following bug has been observed with kernel 7.1.3 after adding a new
rdev to an existing RAID1 array that had enabled serialize_policy:

[ 1754.091836][T19639] Oops: Oops: 0002 [#1] SMP NOPTI
[ 1754.092679][T19639] CPU: 0 UID: 0 PID: 19639 Comm: ext4lazyinit Not tainted 7.1.3-1-default #1 PREEMPT(full) openSUSE Tumbleweed  b041a6527f6e58424f4cd3de0fade8d408b378fd
[ 1754.095245][T19639] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS unknown 02/02/2022
[ 1754.096772][T19639] RIP: 0010:_raw_spin_lock_irqsave+0x27/0x50
[ 1754.097769][T19639] Code: 90 90 90 f3 0f 1e fa 0f 1f 44 00 00 53 9c 58 0f 1f 40 00 48 89 c3 fa 0f 1f 44 00 00 65 ff 05 c0 bc d8 01 31 c0 ba 01 00 00 00 <3e> 0f b1 17 75 09 48 89 d8 5b e9 85 8d fc fe 89 c6 e8 c3 08 00 00
[ 1754.100996][T19639] RSP: 0018:ffffcfbbc0a5bab8 EFLAGS: 00010046
[ 1754.102037][T19639] RAX: 0000000000000000 RBX: 0000000000000212 RCX: 61c8864680b583eb
[ 1754.103367][T19639] RDX: 0000000000000001 RSI: 000000000000092a RDI: 0000000000004960
[ 1754.104723][T19639] RBP: ffff8efae2b12700 R08: 00000000ffffffff R09: 0000000000000000
[ 1754.106038][T19639] R10: ffffffffa0605ac0 R11: 0000000000000001 R12: ffff8efad4199400
[ 1754.107343][T19639] R13: 0000000000004950 R14: ffff8efaf9164600 R15: 000000000004043e
[ 1754.108697][T19639] FS:  0000000000000000(0000) GS:ffff8efbdc754000(0000) knlGS:0000000000000000
[ 1754.110190][T19639] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1754.111280][T19639] CR2: 0000000000004960 CR3: 000000000d830001 CR4: 0000000000770ef0
[ 1754.112580][T19639] PKRU: 55555554
[ 1754.113182][T19639] Call Trace:
[ 1754.113754][T19639]  <TASK>
[ 1754.114256][T19639]  wait_for_serialization+0xb9/0x260 [raid1 9ad4618e4becc97c5a7140f7aa35cc4730d97dd9]
[ 1754.115869][T19639]  raid1_make_request+0x762/0xaff [raid1 9ad4618e4becc97c5a7140f7aa35cc4730d97dd9]
[ 1754.118368][T19639]  md_handle_request+0x1c9/0x2e0 [md_mod a261f96e3d61eb9d00ceb34dc9439cf6487db185]

The raid1.c code calls wait_for_serialization() if the MD_SERIALIZE_POLICY
is set, and wait_for_serialization assumes that rdev->serial is
initialized. Normally this will be the case for arrays that have
the serialize_policy sysfs attribute set to 1.

But when a new rdev is added to an existing array in bind_rdev_to_array(),
the condition at mddev_create_serial_pool() causes creation of rdev->serial
to be skipped. Fix it.

Fixes: 69b00b5bb235 ("md: introduce a new struct for IO serialization")
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 drivers/md/md.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index d1465bcd86c8..baf79c501c9e 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -234,7 +234,8 @@ void mddev_create_serial_pool(struct mddev *mddev, struct md_rdev *rdev)
 {
 	int ret = 0;
 
-	if (rdev && !rdev_need_serial(rdev) &&
+	if (!test_bit(MD_SERIALIZE_POLICY, &mddev->flags) &&
+	    rdev && !rdev_need_serial(rdev) &&
 	    !test_bit(CollisionCheck, &rdev->flags))
 		return;
 
-- 
2.51.0

Re: [PATCH] md/raid1: create serial pool adding rdev to array with serialize_policy=1

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but `mddev_create_serial_pool` silently ignores memory allocation failures, leading to a guaranteed kernel panic on subsequent I/O.
- [High] This problem wasn't introduced by this patch, but echoing `-writemostly` to a device's sysfs state unconditionally destroys its serialization structures, even if the array-wide `MD_SERIALIZE_POLICY` is active, causing a NULL pointer dereference on the next I/O.
--

--- Patch [1]: [PATCH] md/raid1: create serial pool adding rdev to array with serialize_policy=1 ---
commit ac16670aca465de3c72a1f91b7b1bbb4e463a255
Author: Martin Wilck <martin.wilck@suse.com>

md/raid1: create serial pool adding rdev to array with serialize_policy=1
    
The raid1.c code calls wait_for_serialization() if the MD_SERIALIZE_POLICY
is set, and wait_for_serialization assumes that rdev->serial is
initialized. Normally this will be the case for arrays that have
the serialize_policy sysfs attribute set to 1.
    
But when a new rdev is added to an existing array in bind_rdev_to_array(),
the condition at mddev_create_serial_pool() causes creation of rdev->serial
to be skipped. Fix it.
    
Fixes: 69b00b5bb235 ("md: introduce a new struct for IO serialization")
Signed-off-by: Martin Wilck <mwilck@suse.com>

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d1465bcd86c81..baf79c501c9ec 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -234,7 +234,8 @@ void mddev_create_serial_pool(struct mddev *mddev, struct md_rdev *rdev)
>  {
>  	int ret = 0;
>  
> -	if (rdev && !rdev_need_serial(rdev) &&
> +	if (!test_bit(MD_SERIALIZE_POLICY, &mddev->flags) &&
> +	    rdev && !rdev_need_serial(rdev) &&
>  	    !test_bit(CollisionCheck, &rdev->flags))
>  		return;
>  

[Severity: High]
This isn't a bug introduced by this patch, but I noticed that
mddev_create_serial_pool() silently ignores memory allocation failures.

If rdev_init_serial() or mempool_create_kmalloc_pool() fails due to memory
pressure, mddev_create_serial_pool() catches the error but returns early
without propagating it:

drivers/md/md.c:mddev_create_serial_pool() {
	...
	if (ret)
		return;

	if (mddev->serial_info_pool == NULL) {
	...
}

Callers like bind_rdev_to_array() proceed as if successful, setting the
MD_SERIALIZE_POLICY bit. On the next I/O, raid1_make_request() invokes
serialization functions like check_and_add_serial() that unconditionally
dereference the unallocated rdev->serial pointers:

drivers/md/raid1.c:check_and_add_serial() {
	...
	struct serial_in_rdev *serial = &rdev->serial[idx];
	spin_lock_irqsave(&serial->serial_lock, flags);
	...
}

Should mddev_create_serial_pool() return an error code instead of void to
prevent this crash?

[Severity: High]
This is another pre-existing issue, but while reviewing the serialization
logic I noticed that echoing "-writemostly" to a device's state via sysfs
unconditionally destroys its serialization structures.

In state_store(), this destroys the serial pool for the specific rdev without
checking the array-wide policy:

drivers/md/md.c:state_store() {
	...
	} else if (cmd_match(buf, "-writemostly")) {
		mddev_destroy_serial_pool(rdev->mddev, rdev);
		clear_bit(WriteMostly, &rdev->flags);
	...
}

This unconditionally frees rdev->serial for the device:

drivers/md/md.c:mddev_destroy_serial_pool() {
	...
	if (rdev)
		rdev_uninit_serial(rdev);
	...
}

If the array-wide MD_SERIALIZE_POLICY is still active, the next I/O will
invoke wait_for_serialization() and remove_serial(), which dereference the
now-freed rdev->serial pointer:

drivers/md/raid1.c:check_and_add_serial() {
	...
	struct serial_in_rdev *serial = &rdev->serial[idx];
	spin_lock_irqsave(&serial->serial_lock, flags);
	...
}

Should state_store() check if MD_SERIALIZE_POLICY is still active before
destroying the serial pool?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723112741.1206836-1-mwilck@suse.com?part=1

Re: [PATCH] md/raid1: create serial pool adding rdev to array with serialize_policy=1

[Mykola Marzhan]

This reply was drafted with AI assistance.

> But when a new rdev is added to an existing array in bind_rdev_to_array(),
> the condition at mddev_create_serial_pool() causes creation of rdev->serial
> to be skipped. Fix it.

I ran into the same early return from the other direction, so I can
confirm the analysis: the new rdev fails rdev_need_serial() (it is not
a write-mostly member with write-behind configured), CollisionCheck
was never set on it, and mddev_create_serial_pool() returns without
allocating rdev->serial. The first write routed to it then crashes in
wait_for_serialization(), which is gated on the array-wide flag and
assumes the tree exists. serialize_policy is array-wide, so creating
the pool for every rdev bound while it is set is the right fix.

Reviewed-by: Mykola Marzhan <mykola@meshstor.io>

One interaction to flag. Patch 3 of my P2PDMA series changes that
raid1 gate from MD_SERIALIZE_POLICY to per-rdev CollisionCheck, so
that writes which skip write-behind also order against in-flight
behind I/O on write-mostly members (the behind path already checks
CollisionCheck; the non-behind path does not):

  https://lore.kernel.org/linux-raid/20260726113229.2154269-4-mykola@meshstor.io/

For the rdev in your report the two gates disagree: without your fix
it has no serial tree, and the CollisionCheck gate would skip
serialization instead of crashing, hiding the oops but not the bug.
So your patch is needed either way and should go in first; with it,
every rdev in a serialize_policy array owns a serial tree, and the
CollisionCheck gate covers everything the old one did. Patch 3
carries a Link: to your fix.

If you see a hole in that reasoning I would rather hear it now than
after either patch lands.

Mykola

Re: [PATCH] md/raid1: create serial pool adding rdev to array with serialize_policy=1

[Martin Wilck]

On Sun, 2026-07-26 at 14:13 +0200, Mykola Marzhan wrote:
> This reply was drafted with AI assistance.
> 
> > But when a new rdev is added to an existing array in
> > bind_rdev_to_array(),
> > the condition at mddev_create_serial_pool() causes creation of
> > rdev->serial
> > to be skipped. Fix it.
> 
> I ran into the same early return from the other direction, so I can
> confirm the analysis: the new rdev fails rdev_need_serial() (it is
> not
> a write-mostly member with write-behind configured), CollisionCheck
> was never set on it, and mddev_create_serial_pool() returns without
> allocating rdev->serial. The first write routed to it then crashes in
> wait_for_serialization(), which is gated on the array-wide flag and
> assumes the tree exists. serialize_policy is array-wide, so creating
> the pool for every rdev bound while it is set is the right fix.
> 
> Reviewed-by: Mykola Marzhan <mykola@meshstor.io>
> 
> One interaction to flag. Patch 3 of my P2PDMA series changes that
> raid1 gate from MD_SERIALIZE_POLICY to per-rdev CollisionCheck, so
> that writes which skip write-behind also order against in-flight
> behind I/O on write-mostly members (the behind path already checks
> CollisionCheck; the non-behind path does not):
> 
>  
> https://lore.kernel.org/linux-raid/20260726113229.2154269-4-mykola@meshstor.io/
> 
> For the rdev in your report the two gates disagree: without your fix
> it has no serial tree, and the CollisionCheck gate would skip
> serialization instead of crashing, hiding the oops but not the bug.
> So your patch is needed either way and should go in first; with it,
> every rdev in a serialize_policy array owns a serial tree, and the
> CollisionCheck gate covers everything the old one did. Patch 3
> carries a Link: to your fix.
> 
> If you see a hole in that reasoning I would rather hear it now than
> after either patch lands.

I fully agree. My patch is just a small bug fix. Your changes go much
deeper.

Regards,
Martin

-- 
Dr. Martin Wilck <mwilck@suse.com>
SUSE Software Solutions Germany GmbH, Frankenstr. 146, 90461 Nürnberg,
Germany
Geschäftsführer: Jochen Jaser, Andrew McDonald (HRB 36809,AG Nürnberg)