Re: [PATCH bpf v2 07/15] bpf, sockmap: Check the target of an egress redirect before each send

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

On 11/2/26 3:21 PM, Alexei Starovoitov wrote:
> From: Chris Mason <mason@kernel.org>
>
> tcp_sendmsg_locked() expects a caller that has the socket open. It uses
> sk->sk_socket and the wait queue sk->sk_wq. For an egress redirect of an
> sk_msg prog the sender calls it on the target without a reference to the
> target's file. Both pointers are NULL before accept() and after close().
> close() can occur while the sender sleeps. Then the kernel crashes.
>
> Fail the redirect when the target has no struct socket, or when the sender
> has to wait for the connection. The error is EINVAL. After close() it is
> EPIPE, as today. sk_stream_wait_connect() calls remove_wait_queue() without
> a test of SOCK_DEAD. sk_stream_wait_memory() has this test, but it can
> still use sk_wq after the sleep. This patch does not change that. The
> target can change while the sender sleeps there. Thus do both checks before
> each call of tcp_sendmsg_locked(). Clear MSG_FASTOPEN, so that the sender
> does not call the connect code of the target, which uses both pointers.
>
> Today the sender can send before accept() and waits in SYN_SENT. send()
> with MSG_FASTOPEN sends the data. Today it fails with EISCONN. When the
> redirect of a cork fails, send() returns success and the kernel drops the
> data, as today.
>
> The Fixes tags are for the two checks and for MSG_FASTOPEN, in this order.
>
> Fixes: 4f738adba30a ("bpf: create tcp_bpf_ulp allowing BPF to monitor socket TX/RX data")
> Fixes: b8b8315e39ff ("bpf, sockmap: Remove unhash handler for BPF sockmap usage")
> Fixes: c5c37af6ecad ("tcp: Convert do_tcp_sendpages() to use MSG_SPLICE_PAGES")
> Signed-off-by: Chris Mason <mason@kernel.org>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  net/ipv4/tcp_bpf.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
>
> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index fd443350f9b8..ef2e15de0fe3 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
> @@ -88,6 +88,24 @@ static int bpf_tcp_ingress(struct sock *sk, struct sk_psock *psock,
>  	return ret;
>  }
>  
> +/*
> + * The target of a redirect belongs to somebody else. A child that nobody has
> + * accepted yet, and an orphan, have no struct socket and no wait queue.
> + * tcp_sendmsg_locked() needs both. See sendmsg_locked() in skbuff.c.
> + * The sender holds no reference to the file of the target, which can be closed
> + * whenever tcp_sendmsg_locked() sleeps. sk_stream_wait_connect() and
> + * inet_wait_for_connect() go back to the wait queue when they wake up, so the
> + * sender must not wait for a connect.
> + */
> +static bool tcp_bpf_redir_allowed(const struct sock *sk)
> +{
> +	if (!sk->sk_socket)
> +		return false;
> +	if ((1 << sk->sk_state) & (TCPF_SYN_SENT | TCPF_SYN_RECV))
> +		return tcp_passive_fastopen(sk);
> +	return true;
> +}
> +
>  static int tcp_bpf_push(struct sock *sk, struct sk_msg *msg, u32 apply_bytes,
>  			int flags, bool uncharge)
>  {
> @@ -109,6 +127,10 @@ static int tcp_bpf_push(struct sock *sk, struct sk_msg *msg, u32 apply_bytes,
>  		page = sg_page(sge);
>  
>  retry:
> +		/* Only a redirect sends data which is not charged to sk */
> +		if (!uncharge && unlikely(!tcp_bpf_redir_allowed(sk)))
> +			return sock_flag(sk, SOCK_DEAD) ? -EPIPE : -EINVAL;
> +


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

I can trigger it by redirecting data to a non-accept sk.

==================================================================
BUG: KASAN: null-ptr-deref in tcp_sendmsg_locked+0xfb8/0x4550
Write of size 8 at addr 0000000000000008 by task repro/488
Tainted: [W]=WARN
Call Trace:
 <TASK>
 dump_stack_lvl+0x91/0xf0
 print_report+0x442/0x630
 kasan_report+0xf3/0x130
 kasan_check_range+0x11c/0x200
 __kasan_check_write+0x14/0x30
 tcp_sendmsg_locked+0xfb8/0x4550
 tcp_bpf_push+0x3d0/0xbc0
 tcp_bpf_sendmsg_redir+0x639/0x1050
 tcp_bpf_sendmsg+0xa32/0x1b90
 inet_sendmsg+0xdf/0x100
 __sys_sendto+0x3ed/0x4f0
 __x64_sys_sendto+0xe0/0x1b0
 x64_sys_call+0x1375/0x27e0
 do_syscall_64+0xe5/0x610