Re: [PATCH bpf 1/4] bpf, sockmap: reject overflowing copy + len in bpf_msg_push_data()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Out-of-bounds memory access and scatterlist corruption when appending to a full or nearly full ring buffer.
--

commit 98c76ef1269e7a7079e13de8f22677af2c26b2dd
Author: Weiming Shi <bestswngs@gmail.com>

bpf, sockmap: reject overflowing copy + len in bpf_msg_push_data()

This commit adds an overflow check to bpf_msg_push_data() when calculating
the page allocation size. It prevents an integer overflow where a crafted
length could wrap the copy + len sum, leading to an undersized allocation
and a subsequent out-of-bounds memory access.

Link: https://lore.kernel.org/all/20260424155913.A19FDC19425@smtp.kernel.org

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 9590877b0714f..3c8f1cedb217f 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2829,6 +2829,9 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
>  	if (!space || (space == 1 && start != offset))
>  		copy = msg->sg.data[i].length;

[Severity: Critical]
This is a pre-existing issue, but can this read the length of the wrong
scatterlist element in bpf_msg_push_data()?

When appending data to a socket message (start == msg->sg.size), the loop
terminates with i pointing to msg->sg.end. If the scatterlist ring is full
(!space), msg->sg.end wraps around and equals msg->sg.start. This evaluates:

    copy = msg->sg.data[i].length;

which incorrectly reads the length of the first element in the ring buffer
instead of the last.

This allocates a page based on the wrong length, but the coalescing step
steps i back and copies bytes from the actual last element:

    memcpy(raw, from, front);

If the last element is significantly larger than the first, does this cause
an out-of-bounds memcpy that can corrupt adjacent kernel heap memory? Also, if
there is exactly one free slot (space == 1), does copy read the uninitialized
length of that free slot?

>  
> +	if (unlikely(copy + len < copy))
> +		return -EINVAL;
> +
>  	page = alloc_pages(__GFP_NOWARN | GFP_ATOMIC | __GFP_COMP,
>  			   get_order(copy + len));
>  	if (unlikely(!page))

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611013547.247039-1-jiayuan.chen@linux.dev?part=1