Re: [PATCH v4 08/25] md: add helper to split bios at reshape offset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] READ_ONCE() on the 64-bit `mddev->reshape_position` can result in a torn read on 32-bit architectures, leading to incorrect bio splitting and silent data corruption.
--

--- Patch [8]: [PATCH v4 08/25] md: add helper to split bios at reshape offset ---
commit 71f7d05880dacfb8a18a94d6924ba3faac7d7965
Author: Yu Kuai <yukuai@fygo.io>

md: add helper to split bios at reshape offset

This commit adds mddev_bio_split_at_reshape_offset() to allow RAID
personalities to share reshape-offset bio splitting. It introduces a
lockless read of mddev->reshape_position using READ_ONCE().

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
[ ... ]
> +struct bio *mddev_bio_split_at_reshape_offset(struct mddev *mddev,
> +					      struct bio *bio,
> +					      unsigned int *max_sectors,
> +					      struct bio_set *bs)
> +{
> +	sector_t boundary;
> +	sector_t start;
> +	sector_t end;
> +	unsigned int split_sectors;
> +
> +	split_sectors = bio_sectors(bio);
> +	if (max_sectors && *max_sectors && *max_sectors < split_sectors)
> +		split_sectors = *max_sectors;
> +
> +	if (!test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery))
> +		goto split;
> +
> +	boundary = READ_ONCE(mddev->reshape_position);
> +	start = bio->bi_iter.bi_sector;
> +	end = bio_end_sector(bio);
> +	if (start >= boundary || end <= boundary)
> +		goto split;

[Severity: Critical]
Is it possible for the READ_ONCE() above to result in a torn read on 32-bit
architectures?

Since mddev->reshape_position is a sector_t (64-bit value), compiling a
READ_ONCE() on a 32-bit architecture will result in two separate 32-bit loads.
If the reshape thread concurrently updates this value without a lock, a read
could happen exactly when crossing a 32-bit boundary, producing a corrupted
boundary value.

If the torn read produces an oversized boundary value, will this bounds check
evaluate to true and incorrectly skip splitting the bio?

Passing an unsplit bio that spans both old and new geometry layouts to the
RAID personality might result in writes to incorrect disks and offsets. Before
this helper was introduced, did personalities like RAID5 use a lock (like
conf->device_lock) to prevent this 64-bit read race?

> +
> +	if (boundary - start < split_sectors)
> +		split_sectors = boundary - start;
> +
> +split:
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260801172519.2982121-1-yukuai@kernel.org?part=8