Re: [PATCH bpf 1/2] bpf: Fix out-of-bounds read of sk_protocol in bpf_sock_destroy()

[bot+bpf-ci]

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 61940e7535523..1bbb72138ac64 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -12912,8 +12912,8 @@ __bpf_kfunc_start_defs();
>   * @sock: Pointer to socket to be destroyed
>   *
>   * Return:
> - * On error, may return EPROTONOSUPPORT, EINVAL.
> - * EPROTONOSUPPORT if protocol specific destroy handler is not supported.
> + * On error, may return EOPNOTSUPP, EINVAL.
> + * EOPNOTSUPP if protocol specific destroy handler is not supported.
>   * 0 otherwise

The EPROTONOSUPPORT -> EOPNOTSUPP correction is right, but the rewritten
error list is incomplete. tcp_abort() returns -ENOENT when the socket is
already closed (net/ipv4/tcp.c, around the sk_state == TCP_CLOSE check):

    /* Avoid closing the same socket twice. */
    if (sk->sk_state == TCP_CLOSE) {
        if (!has_current_bpf_ctx())
            release_sock(sk);
        return -ENOENT;
    }

That error propagates through sk->sk_prot->diag_destroy(sk, ECONNABORTED)
and is reachable from a BPF iterator (destroying a sock the iterator
already destroyed, or one closed concurrently). In contrast, EINVAL is only
reachable through the narrower tcp_abort_override() path for MPTCP listener
subflows.

Should ENOENT be added to the list?

> @@ -12925,8 +12925,12 @@ __bpf_kfunc int bpf_sock_destroy(struct sock_common *sock)
>  	 * Supporting protocols will need to acquire sock lock in the BPF context
>  	 * prior to invoking this kfunc.
>  	 */
> -	if (!sk->sk_prot->diag_destroy || (sk->sk_protocol != IPPROTO_TCP &&
> -					   sk->sk_protocol != IPPROTO_UDP))
> +	if (!sk->sk_prot->diag_destroy)
> +		return -EOPNOTSUPP;
> +
> +	if (sk_fullsock(sk) &&
> +	    sk->sk_protocol != IPPROTO_TCP &&
> +	    sk->sk_protocol != IPPROTO_UDP)
>  		return -EOPNOTSUPP;
>
>  	return sk->sk_prot->diag_destroy(sk, ECONNABORTED);


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/33760037892