Re: [PATCH 10/13] block: don't require data unit aligned bio_vecs for encrypted bios

[Baokun Li <libaokun@linux.alibaba.com>]

Added David, Chris and the btrfs list.


On 2026/10/12 01:24, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Critical] btrfs: infinite loop and double submission in btrfs_submit_bbio() due to unhandled negative error from bio_split_rw_at()
> --
>
> [Severity: Critical]
> Since bio_split_io_at() (and functions wrapping it like bio_split_rw_at())
> returns negative error codes such as -EINVAL, can this lead to an infinite
> loop in btrfs callers?
>
> In fs/btrfs/bio.c:btrfs_append_map_length(), the return value is not
> checked for negative error codes:
>
> 	sector_offset = bio_split_rw_at(&bbio->bio, &fs_info->limits,
> 					&nr_segs, map_length);
> 	if (sector_offset) {
> 		/* ... */
> 		return ALIGN_DOWN(sector_offset << SECTOR_SHIFT,
> 				  fs_info->sectorsize);
> 	}
>
> If bio_split_rw_at() returns a negative error code (such as -EINVAL or
> -EAGAIN), ALIGN_DOWN() will operate on a negative value and return a massive
> u64.
>
> This huge value is then returned as map_length to btrfs_submit_chunk(),
> where it causes the bio split to be skipped since map_length < length is
> false.
>
> Because btrfs_submit_chunk() returns false, it causes its caller
> btrfs_submit_bbio() to enter an infinite loop:
>
> 	while (!btrfs_submit_chunk(bbio, mirror_num))
> 		;
>
> Do the btrfs callers need to be updated to handle these negative return
> values, or does this patch change the conditions under which
> bio_split_io_at() returns an error for encrypted bios?
>

This patch doesn't change when bio_split_io_at() returns an error.
For bios without a crypto context start_align_mask was already
lim->dma_alignment, and the data unit masks were only added inside
the "if (bc)" branch, so the bvec checks are byte-for-byte identical
before and after this patch.

For encrypted bios the patch only drops the data unit alignment
requirement, so it can make -EINVAL less likely, never more.

The missing negative check in btrfs_append_map_length() is thus a
pre-existing btrfs issue.  It came with fec2e705729d ("block: check
for valid bio while splitting"), which added the -EINVAL returns but
didn't update the btrfs caller, and it is only reachable on the
zoned append path.

The fix should check for a negative return and fail the bio, as
bio_submit_split() does.


Regards,
Baokun