Re: [PATCH bpf] bpf, sockmap: keep sk_msg copy bitmap in sync

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Incorrect page offset calculation when splitting a scatterlist element in bpf_msg_push_data() causes out-of-bounds memory access.
--

commit 68cddb5ca40528f94f0edb5061db37a66f108a6a
Author: Junxi Qian <qjx1298677004@gmail.com>

bpf, sockmap: keep sk_msg copy bitmap in sync

The sk_msg scatterlist ring stores the actual page descriptors in sg.data[]
and stores the copy-required state for each slot in the parallel sg.copy
bitmap. Several SK_MSG helpers move or split sg.data[] entries without
moving the matching sg.copy bit. This patch keeps sg.copy synchronized
whenever these helpers move, split, replace or remove scatterlist entries.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 9590877b0714f..cb8628300ac66 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c

[ ... ]

> @@ -2866,6 +2871,7 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
>  			sk_msg_iter_var_prev(i);
>  		psge = sk_msg_elem(msg, i);
>  		rsge = sk_msg_elem_cpy(msg, i);
> +		rsge_copy = test_bit(i, msg->sg.copy);
>  
>  		psge->length = start - offset;
>  		rsge.length -= psge->length;

[Severity: Critical]
This is a pre-existing issue, but when splitting the scatterlist element here,
does the code immediately following this hunk incorrectly advance the physical
page offset?

net/core/filter.c:bpf_msg_push_data() {
    ...
		psge->length = start - offset;
		rsge.length -= psge->length;
		rsge.offset += start;
    ...
}

Does this incorrectly advance the physical page offset by the absolute byte
offset (start) instead of the fragment's consumed length (start - offset)?

If the element being split is not the first in the message (offset > 0),
could adding the absolute start offset push rsge.offset beyond the valid
bounds of the physical page, leading to an out-of-bounds memory access?

Should this be rsge.offset += start - offset; or rsge.offset += psge->length;
instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520112327.189660-1-qjx1298677004@gmail.com?part=1