Re: [PATCH bpf v2 2/2] bpf: revalidate assigned sockets after protocol change

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] bpf_skb_change_proto can synchronously free a socket via skb_orphan(), leading to a Use-After-Free if the BPF program holds a PTR_TO_SOCK_COMMON pointer to it.
--

commit ecfe32dc3cf3e047fa0e8a7841baab762512d237
Author: Shihuang Liu <shlomojune6@gmail.com>

bpf: revalidate assigned sockets after protocol change

This commit adds a check in bpf_skb_change_proto() to revalidate any
prefetched socket after a protocol change. If the socket's address family
is no longer compatible, it releases the assignment by calling skb_orphan().

Link: https://lore.kernel.org/netdev/20260823101809.26802-1-shlomojune6@gmail.com/

[ ... ]

> diff --git a/net/core/filter.c b/net/core/filter.c
> index b5452113f974a..9907e9d44c11f 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3547,6 +3547,11 @@ BPF_CALL_3(bpf_skb_change_proto, struct sk_buff *, skb, __be16, proto,
>  	if (ret)
>  		return ret;
>  
> +	/* Protocol translation can invalidate an earlier socket assignment. */
> +	if (skb_sk_is_prefetched(skb) &&
> +	    !bpf_sk_assign_family_ok(skb, skb->sk))
> +		skb_orphan(skb);

[Severity: Critical]
Does this code introduce a use-after-free if the BPF program currently holds
a pointer to the socket?

If a BPF program, such as a TC hook, loads skb->sk and verifies it, the
verifier grants a valid PTR_TO_SOCK_COMMON register. When the program
subsequently calls bpf_skb_change_proto(), this newly added code calls
skb_orphan().

The skb_orphan() function synchronously drops the SKB's reference to the
socket via sock_pfree() and sock_gen_put(). If this was the last reference
to an established TCP socket (which do not use SOCK_RCU_FREE), the socket
is freed immediately.

While the verifier's clear_all_pkt_pointers() clears packet data pointers
after protocol changes, it does not invalidate PTR_TO_SOCK_COMMON registers.
Can the BPF program then access the freed socket structure via its dangling
pointer?

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