Re: [PATCH net v2] net: skmsg: preserve sg.copy across SG transforms

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Uninitialized kernel memory is inserted into the socket buffer in `bpf_msg_push_data()`, leading to an info leak.
--

commit 7ed2bff662b24eab5d89d14d0b0a5178d47b9247
Author: Yiming Qian <yimingqian591@gmail.com>
Subject: net: skmsg: preserve sg.copy across SG transforms

Keep sg.copy synchronized with sg.data[] whenever entries are transferred,
shifted, split, or copied into a new sk_msg. Clear the bit when an entry is
replaced by a newly allocated private page or freed.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 80439767e0eea..40037413dd4ec 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c

[ ... ]

> @@ -2837,30 +2841,30 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
>  		copy = msg->sg.data[i].length;
>  
>  	page = alloc_pages(__GFP_NOWARN | GFP_ATOMIC | __GFP_COMP,
>  			   get_order(copy + len));

[Severity: High]
This is a pre-existing issue, but does this allocation lack __GFP_ZERO?

When bpf_msg_push_data() dynamically expands an sk_msg, a new memory
page is allocated here. Without the __GFP_ZERO flag, this page may
contain uninitialized memory.

>  	if (unlikely(!page))
>  		return -ENOMEM;
>  
>  	if (copy) {
>  		int front, back;
>  
>  		raw = page_address(page);
>  
>  		if (i == msg->sg.end)
>  			sk_msg_iter_var_prev(i);
>  		psge = sk_msg_elem(msg, i);
>  		front = start - offset;
>  		back = psge->length - front;
>  		from = sg_virt(psge);
>  
>  		if (front)
>  			memcpy(raw, from, front);
>  
>  		if (back) {
>  			from += front;
>  			to = raw + front + len;
>  
>  			memcpy(to, from, back);
>  		}

[Severity: High]
This is a pre-existing issue, but does this copy logic leave the newly
inserted len bytes completely untouched?

Since the BPF verifier doesn't enforce that dynamically pushed data is
actually written to by the BPF program, these uninitialized bytes can
remain in the buffer.

Because sk_msg structures can be sent over the network or received by
userspace via recvmsg(), can this lead to an information leak of
sensitive kernel memory?

Could this be prevented by zeroing the page initially, similar to how it
is handled in bpf_skb_generic_push() for bpf_skb_adjust_room()?

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