Re: [PATCH bpf v2 13/15] bpf, sockmap: Check that the redirected message is the cork

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

On 11/2/26 3:21 PM, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
>
> In tcp_bpf_send_verdict(), msg is psock->cork or the sk_msg on the stack of
> tcp_bpf_sendmsg(). If psock->cork is not NULL, the SK_REDIRECT branch sets
> it to NULL before release_sock() and calls kfree(msg) after the redirect.
> Thus the branch assumes that a cork is msg.
>
> This is not true after release_sock(), because a second sender on the same
> socket can make a cork. The first sender can run the branch again, and then
> psock->cork is not its msg. Nobody sends or frees the cork of the second
> sender. kfree() gets the sk_msg on the stack, and the kernel crashes.
>
> Compare psock->cork with msg. Nothing changes for one sender, because
> psock->cork is then NULL or msg. The other branches do not change, and a
> cork is still not safe for two senders on one socket.


I think I should try to serialize senders on the same socket. The underlying risk here probably goes beyond this series.


>
> Fixes: 4f738adba30a ("bpf: create tcp_bpf_ulp allowing BPF to monitor socket TX/RX data")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>


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


> ---
>  net/ipv4/tcp_bpf.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index d8e29b09818c..b1071b0960cf 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
> @@ -517,7 +517,11 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
>  			psock->eval = __SK_NONE;
>  			psock->sk_redir = NULL;
>  		}
> -		if (psock->cork) {
> +		/*
> +		 * Another thread can make a cork while this one sends msg
> +		 * without the socket lock. Leave that cork alone.
> +		 */
> +		if (psock->cork == msg) {
>  			cork = true;
>  			psock->cork = NULL;
>  		}