Re: [PATCH 1/2] md: fix bblog_size-driven OOB read in super_1_load() and super_1_sync()

[yu kuai]

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@xxxxxxxxxxxxxxx
> Signed-off-by: zjamg <ndaugoing@xxxxxxxxx>
> ---
>   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