Re: [PATCH bpf 4/4] bpf, sockmap: keep sk_msg copy state in sync

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Uninitialized memory disclosure in bpf_msg_push_data() due to missing __GFP_ZERO.
--

commit 4e239ebdfdc9495774d38523350637e9a010cf74
Author: Zhang Cen <rollkingzzc@gmail.com>

bpf, sockmap: keep sk_msg copy state in sync

This commit fixes an issue where modifying helpers like bpf_msg_push_data()
rewrite the scatterlist ring but leave the copy bitmap unsynchronized. It
keeps the msg->sg.copy bitmap synchronized with scatterlist entry moves to
prevent data corruption.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 982d59cf659f5..95961b4e66a6d 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
[ ... ]
> @@ -2918,13 +2973,18 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
>  	/* Place newly allocated data buffer */
>  	sk_mem_charge(msg->sk, len);
>  	msg->sg.size += len;
> -	__clear_bit(new, msg->sg.copy);
> +	sk_msg_clear_elem_copy(msg, new);
>  	sg_set_page(&msg->sg.data[new], page, len + copy, 0);

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

When the page is allocated earlier in bpf_msg_push_data() via alloc_pages()
without the __GFP_ZERO flag:

    page = alloc_pages(__GFP_NOWARN | GFP_ATOMIC | __GFP_COMP,
                       get_order(copy + len));

The newly inserted payload space of size len is not zeroed. Then it is
added to the scatterlist here via sg_set_page().

Could this lead to an uninitialized memory disclosure over the network?
If copy is 0, the entire page might be left uninitialized. If copy is
greater than 0, the gap of len bytes between the copied front and back
portions might be uninitialized.

Because the BPF verifier cannot enforce that a BPF program initializes this
newly inserted space before the packet is sent, could the kernel transmit
uninitialized page memory?

>  	if (rsge.length) {
>  		get_page(sg_page(&rsge));
>  		sk_msg_iter_var_next(new);
>  		msg->sg.data[new] = rsge;
> +		if (rsge_copy)
> +			sk_msg_set_elem_copy(msg, new);
> +		else
> +			sk_msg_clear_elem_copy(msg, new);
>  	}
> +	sk_msg_clear_elem_copy(msg, msg->sg.end);
[ ... ]

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