mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/2] md: fix bblog_size-driven OOB read in md_write_metadata()
@ 2026-09-17  8:38 zjamg
  2026-09-17  8:38 ` [PATCH 1/2] md: fix bblog_size-driven OOB read in super_1_load() and super_1_sync() zjamg
  2026-09-17  8:38 ` [PATCH 2/2] md: add defensive bounds check for bio in md_write_metadata() zjamg
  0 siblings, 2 replies; 6+ messages in thread
From: zjamg @ 2026-09-17  8:38 UTC (permalink / raw)
  To: Song Liu, Yu Kuai
  Cc: Li Nan, Xiao Ni, NeilBrown, linux-raid, linux-kernel, zjamg

Hi,

This series fixes an out-of-bounds read in the md badblocks log (bblog)
write path. The on-disk bblog_size field is taken verbatim from the
member device superblock and used as the bio length in
md_write_metadata(), without any check against the size of the page that
actually backs the write.

This is a different issue from CVE-2026-89557, which fixed the
bblog_shift overflow. The bblog_size path is still unfixed.

Introduced by commit 2699b67223ac ("md: load/store badblock list from v1.x metadata").

Mechanism:

  super_1_load() reads sb->bblog_size (__le16) from the member device
  and stores it in rdev->badblocks.size via super_1_sync() (md.c:2316).
  md_update_sb() then calls

      md_write_metadata(mddev, rdev, rdev->badblocks.sector,
                        rdev->badblocks.size << 9, rdev->bb_page, 0);

  (md.c:2944-2948). bb->size is sector_t, so 0xFFFF << 9 = 33553920
  does not wrap. md_write_metadata() passes that length to
  __bio_add_page() with a single 4 KiB page:

      __bio_add_page(bio, page, size, 0)

  __bio_add_page() is the no-check variant: after a WARN_ON_ONCE() it
  unconditionally records bvec_set_page(page, size, offset) and bumps
  bi_size/bi_vcnt. bio_full()'s second clause is
  "bi_size > BIO_MAX_SIZE - len", with bi_size == 0 and
  BIO_MAX_SIZE == UINT_MAX, so it is false and not even the WARN fires.
  The 4 KiB page is recorded as a bvec with bv_len == 33553920.

Downstream bvec iteration (nth_page, bio_split_to_limits, etc.) then
reads past the end of the page, into whatever follows in the linear
map. KASAN reports slab-out-of-bounds or slab-use-after-free, and the
data is written to the attacker-controlled member device.

Reproducer (CONTROL/ATTACK, only bblog_size differs):

  CONTROL bblog_size = 8      -> 4096 bytes land, out-of-range stays
                                 at the fill pattern
  ATTACK  bblog_size = 0xFFFF -> 33553920 bytes land, KASAN reports
                                 slab-out-of-bounds in
                                 copy_folio_from_iter_atomic(), and
                                 ~33.5 MB of kernel memory is copied
                                 into the member device image

The harness creates the on-disk superblock by hand, attaches the loop
device, binds it to a new md array via sysfs, adds one bad block, and
then writes "writemostly" to the member state attribute to force an
md_update_sb(). Both runs share the same kernel, the same md module and
the same code path; the only difference is the bblog_size field in the
on-disk superblock.

Patch 1 adds an upper bound for bb->size in super_1_sync(), matching
the existing guard in super_1_load().

Patch 2 is a defensive check in md_write_metadata() itself, so that
any future caller that passes an oversized length is caught before the
bio is built. Only one of the two is strictly needed to fix the
immediate issue; I'm sending both because the checks cover different
layers.

I can send the harness and full dmesg logs if useful.

Thanks.


zjamg (2):
  md: fix bblog_size-driven OOB read in super_1_load() and
    super_1_sync()
  md: add defensive bounds check for bio in md_write_metadata()

 drivers/md/md.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] md: fix bblog_size-driven OOB read in super_1_load() and super_1_sync()
  2026-09-17  8:38 [PATCH 0/2] md: fix bblog_size-driven OOB read in md_write_metadata() zjamg
@ 2026-09-17  8:38 ` zjamg
  2026-09-20 11:50   ` yu kuai
  2026-09-17  8:38 ` [PATCH 2/2] md: add defensive bounds check for bio in md_write_metadata() zjamg
  1 sibling, 1 reply; 6+ messages in thread
From: zjamg @ 2026-09-17  8:38 UTC (permalink / raw)
  To: Song Liu, Yu Kuai
  Cc: Li Nan, Xiao Ni, NeilBrown, linux-raid, linux-kernel, zjamg, stable

In super_1_load(), when the on-disk feature_map does not have the
MD_FEATURE_BAD_BLOCKS flag set, but sb->bblog_offset is non-zero,
rdev->badblocks.shift is initialized to 0 while completely bypassing
the sectors > (PAGE_SIZE / 512) validation check on sb->bblog_size.

Subsequently, when badblocks are updated, super_1_sync() enables the
MD_FEATURE_BAD_BLOCKS feature flag and assigns bb->size directly from
sb->bblog_size without validating that bb->size fits within the single
allocated rdev->bb_page (which is PAGE_SIZE, or PAGE_SIZE / 512 sectors).

When md_update_sb() later writes metadata via md_write_metadata(), an
unvalidated bb->size (e.g. 0xFFFF) results in a bio whose length exceeds
the backing page, causing out-of-bounds reads into kernel memory during
block I/O and leaking kernel slab/page contents to disk.

Fix this by:
1. Validating sb->bblog_size in the 'else if (sb->bblog_offset != 0)'
   branch of super_1_load(), returning -EINVAL on oversized values.
2. Clamping bb->size in super_1_sync() to PAGE_SIZE / 512 to ensure it
   never exceeds the backing rdev->bb_page capacity.

Fixes: 2699b67223ac ("md: load/store badblock list from v1.x metadata")
Cc: stable@vger.kernel.org
Signed-off-by: zjamg <ndaugoing@gmail.com>
---
 drivers/md/md.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 680b34a63cb3..d2433cf41e65 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1934,8 +1934,11 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
 			if (!badblocks_set(&rdev->badblocks, sector, count, 1))
 				return -EINVAL;
 		}
-	} else if (sb->bblog_offset != 0)
+	} else if (sb->bblog_offset != 0) {
+		if (le16_to_cpu(sb->bblog_size) > (PAGE_SIZE / 512))
+			return -EINVAL;
 		rdev->badblocks.shift = 0;
+	}
 
 	if ((le32_to_cpu(sb->feature_map) &
 	    (MD_FEATURE_PPL | MD_FEATURE_MULTIPLE_PPLS))) {
@@ -2313,7 +2316,8 @@ static void super_1_sync(struct mddev *mddev, struct md_rdev *rdev)
 
 			bb->sector = (rdev->sb_start +
 				      (int)le32_to_cpu(sb->bblog_offset));
-			bb->size = le16_to_cpu(sb->bblog_size);
+			bb->size = min_t(sector_t, le16_to_cpu(sb->bblog_size),
+					 PAGE_SIZE / 512);
 		}
 	}
 
-- 
2.53.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] md: add defensive bounds check for bio in md_write_metadata()
  2026-09-17  8:38 [PATCH 0/2] md: fix bblog_size-driven OOB read in md_write_metadata() zjamg
  2026-09-17  8:38 ` [PATCH 1/2] md: fix bblog_size-driven OOB read in super_1_load() and super_1_sync() zjamg
@ 2026-09-17  8:38 ` zjamg
  1 sibling, 0 replies; 6+ messages in thread
From: zjamg @ 2026-09-17  8:38 UTC (permalink / raw)
  To: Song Liu, Yu Kuai
  Cc: Li Nan, Xiao Ni, NeilBrown, linux-raid, linux-kernel, zjamg

md_write_metadata() allocates a single-bvec bio via bio_alloc_bioset()
with nr_vecs = 1 and attaches a single page 'page' (either rdev->sb_page
or rdev->bb_page) via __bio_add_page(bio, page, size, offset).

Because the bio can only ever represent a single backing page, passing
a non-positive size or a length where offset + size > PAGE_SIZE is invalid
and can lead to out-of-bounds reads during I/O submission.

Add a defensive check in md_write_metadata() so that any oversized or
invalid request triggers WARN_ON_ONCE() and aborts before constructing
the bio.

Signed-off-by: zjamg <ndaugoing@gmail.com>
---
 drivers/md/md.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index d2433cf41e65..3fda2965c631 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1142,6 +1142,9 @@ void md_write_metadata(struct mddev *mddev, struct md_rdev *rdev,
 	if (test_bit(Faulty, &rdev->flags))
 		return;
 
+	if (WARN_ON_ONCE(size <= 0 || offset + size > PAGE_SIZE))
+		return;
+
 	bio = bio_alloc_bioset(rdev->meta_bdev ? rdev->meta_bdev : rdev->bdev,
 			      1,
 			      REQ_OP_WRITE | REQ_SYNC | REQ_IDLE | REQ_META
-- 
2.53.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] md: fix bblog_size-driven OOB read in super_1_load() and super_1_sync()
  2026-09-17  8:38 ` [PATCH 1/2] md: fix bblog_size-driven OOB read in super_1_load() and super_1_sync() zjamg
@ 2026-09-20 11:50   ` yu kuai
  2026-09-22  4:45     ` Yuchao Zhang
  2026-09-22  4:46     ` [PATCH v2] md: validate bblog_size when loading v1.x badblocks metadata Yuchao Zhang
  0 siblings, 2 replies; 6+ messages in thread
From: yu kuai @ 2026-09-20 11:50 UTC (permalink / raw)
  To: zjamg, Song Liu, yu kuai
  Cc: Li Nan, Xiao Ni, NeilBrown, linux-raid, linux-kernel, stable

Hi,

在 2026/9/17 16:38, zjamg 写道:
> In super_1_load(), when the on-disk feature_map does not have the
> MD_FEATURE_BAD_BLOCKS flag set, but sb->bblog_offset is non-zero,
> rdev->badblocks.shift is initialized to 0 while completely bypassing
> the sectors > (PAGE_SIZE / 512) validation check on sb->bblog_size.

How can this happen? If this is just raw disk metadata inject failure,
just add a checking in super_1_load() is enough.

>
> Subsequently, when badblocks are updated, super_1_sync() enables the
> MD_FEATURE_BAD_BLOCKS feature flag and assigns bb->size directly from
> sb->bblog_size without validating that bb->size fits within the single
> allocated rdev->bb_page (which is PAGE_SIZE, or PAGE_SIZE / 512 sectors).
>
> When md_update_sb() later writes metadata via md_write_metadata(), an
> unvalidated bb->size (e.g. 0xFFFF) results in a bio whose length exceeds
> the backing page, causing out-of-bounds reads into kernel memory during
> block I/O and leaking kernel slab/page contents to disk.
>
> Fix this by:
> 1. Validating sb->bblog_size in the 'else if (sb->bblog_offset != 0)'
>     branch of super_1_load(), returning -EINVAL on oversized values.
> 2. Clamping bb->size in super_1_sync() to PAGE_SIZE / 512 to ensure it
>     never exceeds the backing rdev->bb_page capacity.
>
> Fixes: 2699b67223ac ("md: load/store badblock list from v1.x metadata")
> Cc: stable@vger.kernel.org
> Signed-off-by: zjamg <ndaugoing@gmail.com>
> ---
>   drivers/md/md.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index 680b34a63cb3..d2433cf41e65 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -1934,8 +1934,11 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
>   			if (!badblocks_set(&rdev->badblocks, sector, count, 1))
>   				return -EINVAL;
>   		}
> -	} else if (sb->bblog_offset != 0)
> +	} else if (sb->bblog_offset != 0) {
> +		if (le16_to_cpu(sb->bblog_size) > (PAGE_SIZE / 512))
> +			return -EINVAL;
>   		rdev->badblocks.shift = 0;
> +	}
>   
>   	if ((le32_to_cpu(sb->feature_map) &
>   	    (MD_FEATURE_PPL | MD_FEATURE_MULTIPLE_PPLS))) {
> @@ -2313,7 +2316,8 @@ static void super_1_sync(struct mddev *mddev, struct md_rdev *rdev)
>   
>   			bb->sector = (rdev->sb_start +
>   				      (int)le32_to_cpu(sb->bblog_offset));
> -			bb->size = le16_to_cpu(sb->bblog_size);
> +			bb->size = min_t(sector_t, le16_to_cpu(sb->bblog_size),
> +					 PAGE_SIZE / 512);
>   		}
>   	}
>   

-- 
Thanks,
Kuai

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] md: fix bblog_size-driven OOB read in super_1_load() and super_1_sync()
  2026-09-20 11:50   ` yu kuai
@ 2026-09-22  4:45     ` Yuchao Zhang
  2026-09-22  4:46     ` [PATCH v2] md: validate bblog_size when loading v1.x badblocks metadata Yuchao Zhang
  1 sibling, 0 replies; 6+ messages in thread
From: Yuchao Zhang @ 2026-09-22  4:45 UTC (permalink / raw)
  To: Yu Kuai, Song Liu
  Cc: Xiao Ni, NeilBrown, linux-raid, linux-kernel, stable, Yuchao Zhang

Hi Kuai,

Thanks for the review.

The 'else if (sb->bblog_offset != 0)' branch handles the valid on-disk
state where bblog space was reserved at array creation but no bad blocks
have been recorded yet (MD_FEATURE_BAD_BLOCKS clear).  A corrupted or
forged superblock - hardware fault, fuzzing, or a malicious shared disk -
can carry an oversized bblog_size (e.g. 0xFFFF) in that state, bypassing
the existing validation in the MD_FEATURE_BAD_BLOCKS branch.

When the first bad block is later recorded, super_1_sync() copies
sb->bblog_size into bb->size, and md_write_metadata() then builds a bio
larger than the backing rdev->bb_page, reading past it and leaking kernel
memory contents to disk.

You are right that fixing super_1_load() alone is sufficient: the kernel
never modifies sb->bblog_size between load and sync, and with the -EINVAL
check the device never enters the array, so super_1_sync() is never
reached with an oversized value.  v2 keeps only the super_1_load()
validation.

Thanks,
Yuchao

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2] md: validate bblog_size when loading v1.x badblocks metadata
  2026-09-20 11:50   ` yu kuai
  2026-09-22  4:45     ` Yuchao Zhang
@ 2026-09-22  4:46     ` Yuchao Zhang
  1 sibling, 0 replies; 6+ messages in thread
From: Yuchao Zhang @ 2026-09-22  4:46 UTC (permalink / raw)
  To: Yu Kuai, Song Liu
  Cc: Xiao Ni, NeilBrown, linux-raid, linux-kernel, stable, Yuchao Zhang

In super_1_load(), when the on-disk feature_map does not have
MD_FEATURE_BAD_BLOCKS set but sb->bblog_offset is non-zero,
rdev->badblocks.shift is initialized to 0 without validating
sb->bblog_size.  The same field is validated in the
MD_FEATURE_BAD_BLOCKS branch ("sectors > (PAGE_SIZE / 512)"), but this
branch bypasses that check.

A forged on-disk superblock (malicious storage device or crafted
image) can therefore carry an oversized sb->bblog_size.  When bad
blocks are later recorded, super_1_sync() sets bb->size from
sb->bblog_size and md_write_metadata() issues a bio larger than the
single rdev->bb_page, reading beyond the page and leaking kernel
memory contents to disk.

sb->bblog_size is never modified by the kernel between load and sync,
and the MD_FEATURE_BAD_BLOCKS branch already validates it, so checking
it in this branch is sufficient to cover both paths.

Fix this by rejecting an oversized sb->bblog_size with -EINVAL,
matching the existing check in the MD_FEATURE_BAD_BLOCKS branch.

Fixes: 2699b67223ac ("md: load/store badblock list from v1.x metadata")
Cc: stable@vger.kernel.org
Signed-off-by: Yuchao Zhang <ndaugoing@gmail.com>
---
 drivers/md/md.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index 680b34a63cb3..1caa92b40f9a 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1934,8 +1934,11 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
 			if (!badblocks_set(&rdev->badblocks, sector, count, 1))
 				return -EINVAL;
 		}
-	} else if (sb->bblog_offset != 0)
+	} else if (sb->bblog_offset != 0) {
+		if (le16_to_cpu(sb->bblog_size) > (PAGE_SIZE / 512))
+			return -EINVAL;
 		rdev->badblocks.shift = 0;
+	}
 
 	if ((le32_to_cpu(sb->feature_map) &
 	    (MD_FEATURE_PPL | MD_FEATURE_MULTIPLE_PPLS))) {
-- 
2.53.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-09-22  4:46 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-17  8:38 [PATCH 0/2] md: fix bblog_size-driven OOB read in md_write_metadata() zjamg
2026-09-17  8:38 ` [PATCH 1/2] md: fix bblog_size-driven OOB read in super_1_load() and super_1_sync() zjamg
2026-09-20 11:50   ` yu kuai
2026-09-22  4:45     ` Yuchao Zhang
2026-09-22  4:46     ` [PATCH v2] md: validate bblog_size when loading v1.x badblocks metadata Yuchao Zhang
2026-09-17  8:38 ` [PATCH 2/2] md: add defensive bounds check for bio in md_write_metadata() zjamg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®