From: Chris Mason 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 Signed-off-by: Alexei Starovoitov --- 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 aa904ded816f..b635d3ebf4fb 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; + msghdr.msg_flags = flags | MSG_SPLICE_PAGES; has_tx_ulp = tls_sw_has_ctx_tx(sk); if (has_tx_ulp) @@ -547,7 +569,7 @@ static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size) int flags; /* Don't let internal flags through */ - flags = (msg->msg_flags & ~MSG_SENDPAGE_DECRYPTED); + flags = (msg->msg_flags & ~(MSG_SENDPAGE_DECRYPTED | MSG_FASTOPEN)); flags |= MSG_NO_SHARED_FRAGS; psock = sk_psock_get(sk); -- 2.55.0