Re: [PATCH bpf v2 15/15] bpf, sockmap: Clear the end mark of elements added by push and pop

[Jiayuan Chen <jiayuan.chen@linux.dev>]

On 11/2/26 3:21 PM, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
>
> The elements of an sk_msg are in a ring of scatterlist slots. A free slot
> can have the end mark, because sk_msg_init() marks the last slot of the
> ring. Also, sg_init_table() in tcp_bpf_push() marks each emptied slot. Thus
> sk_msg_alloc() and sk_msg_page_add() clear the mark of a new element.
>
> bpf_msg_push_data() and bpf_msg_pop_data() fill a free slot with
> sg_set_page(), which keeps the mark. Then the sk_msg can have an end mark
> before its last element. bpf_tcp_ingress() copies the mark. Since the
> commit below, __sk_msg_recvmsg() stops at the mark and frees the sk_msg.
> The data after the mark is lost, and its pages and memory charge leak.
>
> Clear the mark after the two sg_set_page() calls that can fill a free slot.
> A free slot keeps its mark, as today. tcp_bpf_push() does not read the
> mark, thus SK_PASS and egress redirect do not change.
>
> Fixes: 9974d37ea75f ("skmsg: Fix invalid last sg check in sk_msg_recvmsg()")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>


Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>


> ---
>  net/core/filter.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 50c0cda778db..d68b52150405 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -2995,6 +2995,8 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
>  	msg->sg.size += len;
>  	sk_msg_clear_elem_copy(msg, new);
>  	sg_set_page(&msg->sg.data[new], page, len + copy, 0);
> +	/* A free slot can have the end mark */
> +	sg_unmark_end(&msg->sg.data[new]);
>  	if (rsge.length) {
>  		get_page(sg_page(&rsge));
>  		sk_msg_iter_var_next(new);
> @@ -3128,6 +3130,7 @@ BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start,
>  				sg_set_page(nsge,
>  					    sg_page(sge),
>  					    b, sge->offset + pop + a);
> +				sg_unmark_end(nsge);
>  				sk_msg_set_elem_copy(msg, i, sge_copy);
>  			} else {
>  				struct page *page, *orig;