[PATCH bpf v2 00/15] bpf, sockmap: Fixes for verdicts, redirects, corks and recvmsg()

[PATCH bpf v2 00/15] bpf, sockmap: Fixes for verdicts, redirects, corks and recvmsg()

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

These patches fix bugs in sockmap:

  patches  area
  -------  ----------------------------------------------------------------
  01       recvmsg() after a fault
  02-04    the verdict of an sk_msg prog and the return value of sendmsg()
  05       the backlog of a stopped psock
  06       TCP listeners
  07-08    the target of an egress redirect
  09       the TCP state in recvmsg()
  10-14    corks and the redirect target
  15       bpf_msg_push_data() and bpf_msg_pop_data()

8 of the patches are from Chris Mason and Josef Bacik.

Patch 08 changes net/core/stream.c. Patch 09 moves tcp_can_send_ack() from
net/mptcp/protocol.c to include/net/tcp.h. Patches 10 and 15 change the
sk_msg helpers in net/core/filter.c.

v1 -> v2:
- Patch 03: end the verdict after a failed push too if apply_bytes is 0.
  Another sender can use up apply_bytes during the push (CI bot)
- Patch 04: correct the Fixes tag (CI bot)
- Rebase on the bpf tree

v1: https://lore.kernel.org/bpf/20261101192057.2296489-1-alexei.starovoitov@gmail.com/

Signed-off-by: Alexei Starovoitov <ast@kernel.org>

Alexei Starovoitov (7):
  bpf, sockmap: Don't let an SK_PASS verdict cover the next message
  bpf, sockmap: Subtract all dropped bytes from the return value
  net: Leave sk_stream_wait_memory() once the socket is dead
  bpf, sockmap: Clear the redirect target before the sk_msg prog runs
  bpf, sockmap: Check that the redirected message is the cork
  bpf, sockmap: Keep the rest of the cork after a redirect
  bpf, sockmap: Clear the end mark of elements added by push and pop

Chris Mason (5):
  bpf, sockmap: Give no progs to a TCP listener
  bpf, sockmap: Check the target of an egress redirect before each send
  bpf, sockmap: Check the TCP state in recvmsg()
  bpf, sockmap: Reset the copy position when corking
  bpf, sockmap: Don't copy 0 bytes into a full cork

Josef Bacik (3):
  bpf, sockmap: Write sg.start when recvmsg() faults
  bpf, sockmap: Don't let an SK_DROP verdict stick
  bpf, sockmap: Clear the resume state of a stopped psock

 Documentation/bpf/map_sockmap.rst |  4 +-
 include/linux/skmsg.h             | 19 +++++++
 include/net/tcp.h                 |  6 ++
 net/core/filter.c                 | 17 +-----
 net/core/skmsg.c                  | 13 ++++-
 net/core/sock_map.c               |  9 +++
 net/core/stream.c                 |  6 +-
 net/ipv4/tcp_bpf.c                | 93 ++++++++++++++++++++++++++++---
 net/mptcp/protocol.c              |  6 --
 9 files changed, 141 insertions(+), 32 deletions(-)


base-commit: 5e96760b25fdedbbf9312827f1f237d6b18b03e8
-- 
2.55.0

[PATCH bpf v2 01/15] bpf, sockmap: Write sg.start when recvmsg() faults

[Alexei Starovoitov]

From: Josef Bacik <josef@toxicpanda.com>

__sk_msg_recvmsg() copies an sk_msg to the user buffer and starts at
sg.start. After a full copy of an element it calls put_page(), unless an
skb owns the page. It handles 0 copied bytes as a fault. It writes the new
sg.start after the loop, but not when copy_page_to_iter() faults.

  element    0: length 0, put_page()   1: fault here   2: not copied
  sg.start   ^ today                   ^ this patch

Each subsequent call copies 0 bytes from element 0 and returns -EFAULT. The
user cannot read the rest of the data. sk_msg_free() starts at sg.start
too. It calls put_page() for element 0 again and can free a page in use.

Write sg.start in the fault path too. The commit in the Fixes tag removed
this write. Do not write it for MSG_PEEK, which changes no element.

Fixes: c9c89dcd872e ("bpf, sockmap: Fix partial copy_page_to_iter so progress can still be made")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/skmsg.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index df385a5a961e..f03519468ad1 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -443,6 +443,9 @@ int __sk_msg_recvmsg(struct sock *sk, struct sk_psock *psock, struct msghdr *msg
 			if (copy)
 				copy = copy_page_to_iter(page, sge->offset, copy, iter);
 			if (!copy) {
+				/* The elements in front of i are used up */
+				if (!peek)
+					msg_rx->sg.start = i;
 				copied = copied ? copied : -EFAULT;
 				goto out;
 			}
-- 
2.55.0

[PATCH bpf v2 02/15] bpf, sockmap: Don't let an SK_DROP verdict stick

[Alexei Starovoitov]

From: Josef Bacik <josef@toxicpanda.com>

The verdict of an sk_msg prog is valid for one message, or for the bytes
that bpf_msg_apply_bytes() requests. These bytes can come from many
sendmsg() calls. Thus tcp_bpf_send_verdict() keeps the verdict in
psock->eval and runs the prog only when psock->eval is __SK_NONE. The code
that resets psock->eval is after the switch statement, but the __SK_DROP
branch returns before that code. Then sendmsg() drops all subsequent
messages with -EACCES and does not run the prog.

Reset psock->eval when apply_bytes is 0, as the __SK_REDIRECT branch does.

Fixes: 4f738adba30a ("bpf: create tcp_bpf_ulp allowing BPF to monitor socket TX/RX data")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/ipv4/tcp_bpf.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index 8cf63a9e7fed..d90bcc911b36 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -509,6 +509,9 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
 	default:
 		sk_msg_free(sk, msg);
 		sk_msg_apply_bytes(psock, tosend);
+		/* The verdict holds for apply_bytes, not for ever */
+		if (!psock->apply_bytes)
+			psock->eval = __SK_NONE;
 		*copied -= (tosend + delta);
 		return -EACCES;
 	}
-- 
2.55.0

[PATCH bpf v2 03/15] bpf, sockmap: Don't let an SK_PASS verdict cover the next message

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

tcp_bpf_send_verdict() keeps the verdict of the sk_msg prog in psock->eval
and runs the prog only when eval is __SK_NONE. For __SK_PASS it resets eval
only after tcp_bpf_push() returns 0. But the push can release the socket
lock in sk_stream_wait_memory(), and it can fail. Then another
tcp_bpf_sendmsg() sends its message and does not run the prog.

With bpf_msg_apply_bytes() the verdict covers apply_bytes bytes. After a
failed push, apply_bytes still includes the sent bytes. Thus the verdict
covers more bytes than the prog requested.

Reset eval before the push when apply_bytes is 0. If this push fails, the
prog now runs for the rest of the message. The same already occurs when a
message does not fit into one sk_msg.

When the push fails, subtract the sent bytes from apply_bytes. When
apply_bytes is not 0, eval stays __SK_PASS during the push, and the prog
does not run for another message, as today. The verdict must still cover
the bytes that a failed push does not send. But such a message also lowers
apply_bytes. Thus, if apply_bytes is 0 after the subtraction, end the
verdict, as after a push that returns 0.

Fixes: 4f738adba30a ("bpf: create tcp_bpf_ulp allowing BPF to monitor socket TX/RX data")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/ipv4/tcp_bpf.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index d90bcc911b36..b016aa419fb2 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -457,8 +457,22 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
 
 	switch (psock->eval) {
 	case __SK_PASS:
+		/* The push may sleep without the socket lock, and it may fail */
+		if (!psock->apply_bytes)
+			psock->eval = __SK_NONE;
+		origsize = msg->sg.size;
 		ret = tcp_bpf_push(sk, msg, tosend, flags, true);
 		if (unlikely(ret)) {
+			/* A part of it may have gone out */
+			sk_msg_apply_bytes(psock, origsize - msg->sg.size);
+			/* Another sender can use up the rest during the push */
+			if (!psock->apply_bytes) {
+				psock->eval = __SK_NONE;
+				if (psock->sk_redir) {
+					sock_put(psock->sk_redir);
+					psock->sk_redir = NULL;
+				}
+			}
 			*copied -= sk_msg_free(sk, msg);
 			break;
 		}
-- 
2.55.0

[PATCH bpf v2 04/15] bpf, sockmap: Subtract all dropped bytes from the return value

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

When an sk_msg prog returns SK_DROP, tcp_bpf_send_verdict() frees the full
message. It then subtracts tosend bytes from *copied, the return value of
sendmsg(). But tosend is the number of bytes that the verdict applies to.
After bpf_msg_apply_bytes() tosend can be less than the message size. Then
sendmsg() reports dropped bytes as sent.

Subtract the number of bytes that sk_msg_free() frees. After an error the
SK_PASS branch already does this. The SK_REDIRECT branch does the same if
the message is not the cork. Nothing changes when tosend is equal to the
message size. Otherwise sendmsg() now returns -EACCES if it sends nothing.

Fixes: ca70b8baf2bd ("tcp_bpf: Fix the sk_mem_uncharge logic in tcp_bpf_sendmsg")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/ipv4/tcp_bpf.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index b016aa419fb2..36d1bf46353a 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -521,12 +521,12 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
 		break;
 	case __SK_DROP:
 	default:
-		sk_msg_free(sk, msg);
+		/* All of it is thrown away, not only tosend bytes */
+		*copied -= sk_msg_free(sk, msg) + delta;
 		sk_msg_apply_bytes(psock, tosend);
 		/* The verdict holds for apply_bytes, not for ever */
 		if (!psock->apply_bytes)
 			psock->eval = __SK_NONE;
-		*copied -= (tosend + delta);
 		return -EACCES;
 	}
 
-- 
2.55.0

[PATCH bpf v2 05/15] bpf, sockmap: Clear the resume state of a stopped psock

[Alexei Starovoitov]

From: Josef Bacik <josef@toxicpanda.com>

sk_psock_backlog() passes the skbs of psock->ingress_skb to the socket of
the psock. When the socket has no room, the function saves the length and
offset of the remaining data in psock->work_state. A later run continues at
this offset and clears the state when the skb is complete.

sk_psock_skb_state() saves and clears the state, but only while
SK_PSOCK_TX_ENABLED is set. sk_psock_backlog() does not test this flag for
each skb. If the socket closes during the loop, sk_psock_stop() clears the
flag. The loop continues, but the state of the completed skb stays. The
loop uses this state for all subsequent skbs and sends only part of their
data. A closing socket may lose the end of a stream, but not the middle.

The state belongs to the skb at the head of ingress_skb. Always clear the
state when this skb is complete. A stopped psock still saves no new state.

Fixes: 3b4f14b79428 ("bpf, sockmap: fix duplicated data transmission")
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/skmsg.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index f03519468ad1..a5d498982848 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -664,7 +664,8 @@ static void sk_psock_skb_state(struct sk_psock *psock,
 			       int len, int off)
 {
 	spin_lock_bh(&psock->ingress_lock);
-	if (sk_psock_test_state(psock, SK_PSOCK_TX_ENABLED)) {
+	/* No new state once the psock is stopped, but never keep an old one */
+	if (!len || sk_psock_test_state(psock, SK_PSOCK_TX_ENABLED)) {
 		state->len = len;
 		state->off = off;
 	}
-- 
2.55.0

[PATCH bpf v2 06/15] bpf, sockmap: Give no progs to a TCP listener

[Alexei Starovoitov]

From: Chris Mason <mason@kernel.org>

A psock with a verdict prog replaces sk_data_ready and sk_write_space of
its socket. It saves the old callbacks, and the new callbacks call them.
sock_map_link() does this to a TCP listener too. Its new sk_data_ready
cannot read and does not call the old one. Thus it does not wake accept().

sk_clone() copies the new callbacks to each child, but not the psock.
tcp_bpf_clone() restores only sk_prot. Without a psock the child loses data
or does not wake its reader. If it gets a psock, the psock saves the copies
as the old callbacks. Then sk_psock_verdict_data_ready() calls itself until
the stack overflows. sk_psock_write_space() calls itself too, without end.

Give no progs to a TCP listener, as before the first commit below. A
listener receives and sends no data. It stays in the map and keeps its own
callbacks. Add this exception to map_sockmap.rst. A listener no longer gets
-EBUSY from a second map with progs of the same kind.

Since the second commit below, a socket keeps its psock when TCP unhashes
it. sock_map_link() does not see the state that the socket gets later. Do
not depend on TCP to keep such a socket out of TCP_LISTEN. Set both
callbacks in tcp_bpf_clone() to the TCP defaults.

Fixes: 0c48eefae712 ("sock_map: Lift socket state restriction for datagram sockets")
Fixes: b8b8315e39ff ("bpf, sockmap: Remove unhash handler for BPF sockmap usage")
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 Documentation/bpf/map_sockmap.rst |  4 +++-
 net/core/sock_map.c               |  9 +++++++++
 net/ipv4/tcp_bpf.c                | 15 +++++++++++++--
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/Documentation/bpf/map_sockmap.rst b/Documentation/bpf/map_sockmap.rst
index 2d630686a00b..eb466b86df17 100644
--- a/Documentation/bpf/map_sockmap.rst
+++ b/Documentation/bpf/map_sockmap.rst
@@ -33,7 +33,9 @@ of ``__SK_DROP``, ``__SK_PASS``, or ``__SK_REDIRECT``.
 
 When a socket is inserted into one of these maps, its socket callbacks are
 replaced and a ``struct sk_psock`` is attached to it. Additionally, this
-``sk_psock`` inherits the programs that are attached to the map.
+``sk_psock`` inherits the programs that are attached to the map. A TCP listener
+is an exception. It gets a ``struct sk_psock``, but it keeps its callbacks and
+inherits no programs.
 
 A sock object may be in multiple maps, but can only inherit a single
 parse or verdict program. If adding a sock object to a map would result
diff --git a/net/core/sock_map.c b/net/core/sock_map.c
index efe49551caba..282c6a4b2b05 100644
--- a/net/core/sock_map.c
+++ b/net/core/sock_map.c
@@ -228,6 +228,14 @@ static int sock_map_link(struct bpf_map *map, struct sock *sk)
 	struct sk_psock *psock;
 	int ret;
 
+	/*
+	 * A listener receives no data. With the callbacks of a verdict program
+	 * or a parser it would no longer wake accept(), and its children would
+	 * copy them.
+	 */
+	if (sk_is_tcp(sk) && sk->sk_state == TCP_LISTEN)
+		goto no_progs;
+
 	stream_verdict = READ_ONCE(progs->stream_verdict);
 	if (stream_verdict) {
 		stream_verdict = bpf_prog_inc_not_zero(stream_verdict);
@@ -262,6 +270,7 @@ static int sock_map_link(struct bpf_map *map, struct sock *sk)
 		}
 	}
 
+no_progs:
 	psock = sock_map_psock_get_checked(sk);
 	if (IS_ERR(psock)) {
 		ret = PTR_ERR(psock);
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index 36d1bf46353a..fd443350f9b8 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -785,7 +785,18 @@ void tcp_bpf_clone(const struct sock *sk, struct sock *newsk)
 {
 	struct proto *prot = newsk->sk_prot;
 
-	if (is_insidevar(prot, tcp_bpf_prots))
-		newsk->sk_prot = sk->sk_prot_creator;
+	if (!is_insidevar(prot, tcp_bpf_prots))
+		return;
+
+	newsk->sk_prot = sk->sk_prot_creator;
+	/*
+	 * sock_map_link() gives a listener no programs, but a socket which got
+	 * them in another state and listens now has the psock's sk_data_ready
+	 * and sk_write_space, and sock_copy() has copied them.
+	 * sk_psock_init() refuses a socket that has sk_user_data or a ULP, so
+	 * what the psock replaced were the defaults of a TCP socket.
+	 */
+	newsk->sk_data_ready = sock_def_readable;
+	newsk->sk_write_space = sk_stream_write_space;
 }
 #endif /* CONFIG_BPF_SYSCALL */
-- 
2.55.0

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

[Alexei Starovoitov]

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;
+
 		msghdr.msg_flags = flags | MSG_SPLICE_PAGES;
 		has_tx_ulp = tls_sw_has_ctx_tx(sk);
 		if (has_tx_ulp)
@@ -555,7 +577,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

[PATCH bpf v2 08/15] net: Leave sk_stream_wait_memory() once the socket is dead

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

When an sk_msg prog redirects data to the egress of another socket, the
sender calls tcp_sendmsg_locked() on that socket. The sender holds a
reference to the sock, but not to its file. Thus __tcp_close() can call
sock_orphan() while the sender sleeps in sk_stream_wait_memory().
sock_orphan() sets SOCK_DEAD and sets sk_wq to NULL.

After the sleep the loop continues and does not test SOCK_DEAD. The
subsequent sk_set_bit() can dereference the NULL sk_wq. The tests of sk_err
and sk_shutdown do not always stop the sender. sock_error() clears sk_err,
and __tcp_close() can call tcp_disconnect(), which clears sk_shutdown. The
sender then adds data to the write queue of the dead socket, and
inet_sock_destruct() warns.

Leave the loop with -EPIPE when the socket is dead after the sleep. The
function already tests SOCK_DEAD before remove_wait_queue() for the same
reason. A caller that has the socket open never sees SOCK_DEAD.

Fixes: 4f738adba30a ("bpf: create tcp_bpf_ulp allowing BPF to monitor socket TX/RX data")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/stream.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/core/stream.c b/net/core/stream.c
index cb255e2a3a5d..334124d683a7 100644
--- a/net/core/stream.c
+++ b/net/core/stream.c
@@ -150,7 +150,11 @@ int sk_stream_wait_memory(struct sock *sk, long *timeo_p)
 				    (sk_stream_memory_free(sk) && !vm_wait),
 				    &wait);
 		sk->sk_write_pending--;
-		if (ret < 0)
+		/*
+		 * A sender which sockmap has redirected to sk does not keep it
+		 * open. A dead socket has no sk_wq.
+		 */
+		if (ret < 0 || sock_flag(sk, SOCK_DEAD))
 			goto do_error;
 
 		if (vm_wait) {
-- 
2.55.0

[PATCH bpf v2 09/15] bpf, sockmap: Check the TCP state in recvmsg()

[Alexei Starovoitov]

From: Chris Mason <mason@kernel.org>

With an sk_skb verdict prog, tcp_bpf_recvmsg_parser() copies data from the
psock. Then it calls __tcp_cleanup_rbuf() to calculate the receive window
and to send an ACK. The window and the ACK need a connection on the socket.
But tcp_disconnect() keeps the socket in the sockmap and the data on the
psock. For a TCP socket sock_map_redirect_allowed() rejects only
TCP_LISTEN. Thus the psock can also get data after tcp_disconnect().

__tcp_select_window() divides by icsk_ack.rcv_mss, which can be 0 in
TCP_CLOSE. In TCP_SYN_SENT the new peer can get an ACK with the sequence
numbers of the old connection.

Call __tcp_cleanup_rbuf() only when tcp_can_send_ack() returns true.
recvmsg() returns the data as before. Move tcp_can_send_ack() from MPTCP to
tcp.h. It also returns false for a fastopen child in TCP_SYN_RECV. From its
peer the child has only the SYN data, and the SYN-ACK acknowledges it.

Fixes: e5c6de5fa025 ("bpf, sockmap: Incorrectly handling copied_seq")
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 include/net/tcp.h    | 6 ++++++
 net/ipv4/tcp_bpf.c   | 3 ++-
 net/mptcp/protocol.c | 6 ------
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/net/tcp.h b/include/net/tcp.h
index 3560adddf821..23a22d70718e 100644
--- a/include/net/tcp.h
+++ b/include/net/tcp.h
@@ -1845,6 +1845,12 @@ static inline void tcp_adjust_rcv_ssthresh(struct sock *sk)
 void tcp_cleanup_rbuf(struct sock *sk, int copied);
 void __tcp_cleanup_rbuf(struct sock *sk, int copied);
 
+/* Is the socket in a state in which TCP may send an ACK? */
+static inline bool tcp_can_send_ack(const struct sock *sk)
+{
+	return !((1 << inet_sk_state_load(sk)) &
+	       (TCPF_SYN_SENT | TCPF_SYN_RECV | TCPF_TIME_WAIT | TCPF_CLOSE | TCPF_LISTEN));
+}
 
 /* We provision sk_rcvbuf around 200% of sk_rcvlowat.
  * If 87.5 % (7/8) of the space has been consumed, we want to override
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index ef2e15de0fe3..3d703f1517d8 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -343,7 +343,8 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
 	if (!peek)
 		WRITE_ONCE(tcp->copied_seq, seq);
 	tcp_rcv_space_adjust(sk);
-	if (copied > 0)
+	/* Data on the psock outlives the connection that it came in on */
+	if (copied > 0 && tcp_can_send_ack(sk))
 		__tcp_cleanup_rbuf(sk, copied);
 
 unlock:
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 7a22e9388e0e..d62cfbf2d490 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -619,12 +619,6 @@ static void mptcp_set_timeout(struct sock *sk)
 	__mptcp_set_timeout(sk, tout);
 }
 
-static inline bool tcp_can_send_ack(const struct sock *ssk)
-{
-	return !((1 << inet_sk_state_load(ssk)) &
-	       (TCPF_SYN_SENT | TCPF_SYN_RECV | TCPF_TIME_WAIT | TCPF_CLOSE | TCPF_LISTEN));
-}
-
 void __mptcp_subflow_send_ack(struct sock *ssk)
 {
 	if (tcp_can_send_ack(ssk))
-- 
2.55.0

[PATCH bpf v2 10/15] bpf, sockmap: Reset the copy position when corking

[Alexei Starovoitov]

From: Chris Mason <mason@kernel.org>

sk_msg_memcopy_from_iter() writes to the copy position: byte sg.copybreak
after the offset of element sg.curr. Both tcp_bpf_push() and sk_msg_xfer()
remove bytes from the front of an element. They increase its offset, but
they do not change sg.copybreak when the element is sg.curr.

tcp_bpf_send_verdict() can send a part of a message and keep the rest as
the cork. tcp_bpf_sendmsg() appends new data to it. If sk_msg_alloc() then
extends element sg.curr, the peer can get memory that nobody wrote:

  old offset    offset    end of data     offset + sg.copybreak
  | removed     | rest    | not written   | new data

Move sk_msg_reset_curr() from filter.c to skmsg.h. Call it when a message
becomes the cork, as the helpers that change a message do. It sets the copy
position to the end of the data. Without removed bytes it changes nothing.

Fixes: 91843d540a13 ("bpf: sockmap, add msg_cork_bytes() helper")
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 include/linux/skmsg.h | 19 +++++++++++++++++++
 net/core/filter.c     | 14 --------------
 net/ipv4/tcp_bpf.c    |  5 +++++
 3 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/include/linux/skmsg.h b/include/linux/skmsg.h
index d5e35f24738d..f0e27b4b929f 100644
--- a/include/linux/skmsg.h
+++ b/include/linux/skmsg.h
@@ -196,6 +196,25 @@ static inline void sk_msg_init(struct sk_msg *msg)
 	sg_init_marker(msg->sg.data, NR_MSG_FRAG_IDS);
 }
 
+/*
+ * sk_msg_memcopy_from_iter() writes to byte sg.copybreak of element sg.curr,
+ * counted from the offset of that element. Point both past the last byte of
+ * the message, after something other than a copy has changed its elements.
+ */
+static inline void sk_msg_reset_curr(struct sk_msg *msg)
+{
+	if (!msg->sg.size) {
+		msg->sg.curr = msg->sg.start;
+		msg->sg.copybreak = 0;
+	} else {
+		u32 i = msg->sg.end;
+
+		sk_msg_iter_var_prev(i);
+		msg->sg.curr = i;
+		msg->sg.copybreak = msg->sg.data[i].length;
+	}
+}
+
 static inline void sk_msg_xfer(struct sk_msg *dst, struct sk_msg *src,
 			       int which, u32 size)
 {
diff --git a/net/core/filter.c b/net/core/filter.c
index 9245009d9f1e..50c0cda778db 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2677,20 +2677,6 @@ BPF_CALL_2(bpf_msg_cork_bytes, struct sk_msg *, msg, u32, bytes)
 	return 0;
 }
 
-static void sk_msg_reset_curr(struct sk_msg *msg)
-{
-	if (!msg->sg.size) {
-		msg->sg.curr = msg->sg.start;
-		msg->sg.copybreak = 0;
-	} else {
-		u32 i = msg->sg.end;
-
-		sk_msg_iter_var_prev(i);
-		msg->sg.curr = i;
-		msg->sg.copybreak = msg->sg.data[i].length;
-	}
-}
-
 static bool sk_msg_elem_is_copy(const struct sk_msg *msg, u32 i)
 {
 	return test_bit(i, msg->sg.copy);
diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index 3d703f1517d8..d534974c33f8 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -469,6 +469,11 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
 				return -ENOMEM;
 			}
 		}
+		/*
+		 * A push or a redirect may have taken bytes off the front of
+		 * element sg.curr. The next sendmsg() appends to the cork.
+		 */
+		sk_msg_reset_curr(msg);
 		memcpy(psock->cork, msg, sizeof(*msg));
 		return 0;
 	}
-- 
2.55.0

[PATCH bpf v2 11/15] bpf, sockmap: Don't copy 0 bytes into a full cork

[Alexei Starovoitov]

From: Chris Mason <mason@kernel.org>

psock->cork is a ring. sk_msg_memcopy_from_iter() writes at sg.curr.
sk_msg_alloc() adds room at sg.end, where a full ring has a spare element.

When only a part of the data fits, tcp_bpf_sendmsg() copies this part and
flushes the cork. If nothing fits, it copies 0 bytes. This copy writes
nothing, but it moves sg.curr to sg.end and returns -ENOSPC. Thus sendmsg()
fails without a flush. sg.curr stays at sg.end, and from there a copy moves
it past the spare element. Then a copy can skip the first new element, and
the peer gets memory that nobody wrote.

Skip the copy when sk_msg_alloc() adds nothing, as tls_sw_sendmsg_locked()
does. This pass of the loop flushes the cork. The flush empties the element
at sg.curr, thus the copy in the next pass starts at the first new element.
Now sendmsg() on a full cork does not fail with ENOSPC. The prog runs on
less data than it wants to cork, as it does when a part fits.

The first Fixes tag is for the -ENOSPC without a flush. The commit in the
second tag added the spare element. Since then sg.curr can move past it.

Fixes: 604326b41a6f ("bpf, sockmap: convert to generic sk_msg interface")
Fixes: 031097d9e079 ("net: skmsg: fix TLS 1.3 crash with full sk_msg")
Signed-off-by: Chris Mason <mason@kernel.org>
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/ipv4/tcp_bpf.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index d534974c33f8..d8e29b09818c 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -619,8 +619,15 @@ static int tcp_bpf_sendmsg(struct sock *sk, struct msghdr *msg, size_t size)
 			copy = msg_tx->sg.size - osize;
 		}
 
-		ret = sk_msg_memcopy_from_iter(sk, &msg->msg_iter, msg_tx,
-					       copy);
+		/*
+		 * copy is 0 if the ring of the cork was full already and its
+		 * last element could not be extended. Flush the cork below,
+		 * and let the next pass take the data.
+		 */
+		ret = 0;
+		if (copy)
+			ret = sk_msg_memcopy_from_iter(sk, &msg->msg_iter,
+						       msg_tx, copy);
 		if (ret < 0) {
 			sk_msg_trim(sk, msg_tx, osize);
 			err = ret;
-- 
2.55.0

[PATCH bpf v2 12/15] bpf, sockmap: Clear the redirect target before the sk_msg prog runs

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

bpf_msg_redirect_{map,hash}() store the target in msg->sk_redir, without a
reference. After the prog has run, sk_psock_msg_verdict() makes SK_PASS a
redirect if this field is set. But tcp_bpf_send_verdict() runs the prog
again on a cork, and on the rest of a partly sent message. Nothing clears
the field between runs. Thus SK_PASS without a new target is a redirect to
the old target. If that sock is already free, this is a use after free.

Clear msg->sk_redir before each run, as the skb verdict paths do with
skb_bpf_redirect_clear(). Only SK_PASS without a target changes. It now
sends the data on psock->sk. Then sendmsg() can return another value. A
redirect that applies to more bytes uses psock->sk_redir, with its own
reference. The counts msg->apply_bytes and msg->cork_bytes stay as today.

Fixes: e5cd3abcb31a ("bpf: sockmap, refactor sockmap routines to work with hashmap")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/skmsg.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index a5d498982848..8a3b8536c0c2 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -936,6 +936,13 @@ int sk_psock_msg_verdict(struct sock *sk, struct sk_psock *psock,
 
 	sk_msg_compute_data_pointers(msg);
 	msg->sk = sk;
+	/*
+	 * The prog may have run on this sk_msg before: it is a cork, or the
+	 * rest of a message of which only a part was sent. The target of that
+	 * run is not the target of this run, and the sk_msg holds no reference
+	 * to it.
+	 */
+	msg->sk_redir = NULL;
 	ret = bpf_prog_run_pin_on_cpu(prog, msg);
 	msg->sk = NULL;
 	ret = sk_psock_map_verd(ret, msg->sk_redir);
-- 
2.55.0

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

[Alexei Starovoitov]

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.

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

[PATCH bpf v2 14/15] bpf, sockmap: Keep the rest of the cork after a redirect

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

After bpf_msg_apply_bytes() a verdict can apply to a part of the message.
Then tcp_bpf_send_verdict() sends this part and goes to more_data, and the
sk_msg prog runs on the rest. The SK_REDIRECT branch does not go there if
the message is the cork. It removes the cork from the psock before it
releases the socket lock. After the redirect it frees the cork together
with the rest. Thus the rest is lost, and sendmsg() reports it as sent.

If bytes remain after the redirect, put the cork back into the psock. Then
the prog runs on the rest, as after SK_PASS. If the new verdict is SK_DROP
or the push fails, sendmsg() now returns a smaller value or the error.

msg->cork_bytes keeps its value, thus the rest can stay in the cork. If the
psock has a new cork after lock_sock(), the kernel still frees the rest. If
the redirect of a cork fails, sendmsg() still returns success and frees it.

Fixes: 4f738adba30a ("bpf: create tcp_bpf_ulp allowing BPF to monitor socket TX/RX data")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/ipv4/tcp_bpf.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
index b1071b0960cf..788ce08eef4d 100644
--- a/net/ipv4/tcp_bpf.c
+++ b/net/ipv4/tcp_bpf.c
@@ -544,6 +544,14 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
 			if (!cork)
 				*copied -= free;
 		}
+		/*
+		 * The verdict was for tosend bytes. The rest goes back, unless
+		 * another thread made a new cork while the lock was released.
+		 */
+		if (cork && msg->sg.size && !psock->cork) {
+			psock->cork = msg;
+			cork = false;
+		}
 		if (cork) {
 			sk_msg_free(sk, msg);
 			kfree(msg);
-- 
2.55.0

[PATCH bpf v2 15/15] bpf, sockmap: Clear the end mark of elements added by push and pop

[Alexei Starovoitov]

From: Alexei Starovoitov <ast@kernel.org>

The elements of an sk_msg are in a ring of scatterlist slots. A free slot
can have the end mark, because sk_msg_init() marks the last slot of the
ring. Also, sg_init_table() in tcp_bpf_push() marks each emptied slot. Thus
sk_msg_alloc() and sk_msg_page_add() clear the mark of a new element.

bpf_msg_push_data() and bpf_msg_pop_data() fill a free slot with
sg_set_page(), which keeps the mark. Then the sk_msg can have an end mark
before its last element. bpf_tcp_ingress() copies the mark. Since the
commit below, __sk_msg_recvmsg() stops at the mark and frees the sk_msg.
The data after the mark is lost, and its pages and memory charge leak.

Clear the mark after the two sg_set_page() calls that can fill a free slot.
A free slot keeps its mark, as today. tcp_bpf_push() does not read the
mark, thus SK_PASS and egress redirect do not change.

Fixes: 9974d37ea75f ("skmsg: Fix invalid last sg check in sk_msg_recvmsg()")
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
---
 net/core/filter.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/core/filter.c b/net/core/filter.c
index 50c0cda778db..d68b52150405 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -2995,6 +2995,8 @@ BPF_CALL_4(bpf_msg_push_data, struct sk_msg *, msg, u32, start,
 	msg->sg.size += len;
 	sk_msg_clear_elem_copy(msg, new);
 	sg_set_page(&msg->sg.data[new], page, len + copy, 0);
+	/* A free slot can have the end mark */
+	sg_unmark_end(&msg->sg.data[new]);
 	if (rsge.length) {
 		get_page(sg_page(&rsge));
 		sk_msg_iter_var_next(new);
@@ -3128,6 +3130,7 @@ BPF_CALL_4(bpf_msg_pop_data, struct sk_msg *, msg, u32, start,
 				sg_set_page(nsge,
 					    sg_page(sge),
 					    b, sge->offset + pop + a);
+				sg_unmark_end(nsge);
 				sk_msg_set_elem_copy(msg, i, sge_copy);
 			} else {
 				struct page *page, *orig;
-- 
2.55.0

Re: [PATCH bpf v2 01/15] bpf, sockmap: Write sg.start when recvmsg() faults

[Jiayuan Chen]

On 11/2/26 3:20 PM, Alexei Starovoitov wrote:
> From: Josef Bacik <josef@toxicpanda.com>
>
> __sk_msg_recvmsg() copies an sk_msg to the user buffer and starts at
> sg.start. After a full copy of an element it calls put_page(), unless an
> skb owns the page. It handles 0 copied bytes as a fault. It writes the new
> sg.start after the loop, but not when copy_page_to_iter() faults.
>
>   element    0: length 0, put_page()   1: fault here   2: not copied
>   sg.start   ^ today                   ^ this patch
>
> Each subsequent call copies 0 bytes from element 0 and returns -EFAULT. The
> user cannot read the rest of the data. sk_msg_free() starts at sg.start
> too. It calls put_page() for element 0 again and can free a page in use.
>
> Write sg.start in the fault path too. The commit in the Fixes tag removed
> this write. Do not write it for MSG_PEEK, which changes no element.
>
> Fixes: c9c89dcd872e ("bpf, sockmap: Fix partial copy_page_to_iter so progress can still be made")
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>


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

Re: [PATCH bpf v2 02/15] bpf, sockmap: Don't let an SK_DROP verdict stick

[Jiayuan Chen]

On 11/2/26 3:20 PM, Alexei Starovoitov wrote:
> From: Josef Bacik <josef@toxicpanda.com>
>
> The verdict of an sk_msg prog is valid for one message, or for the bytes
> that bpf_msg_apply_bytes() requests. These bytes can come from many
> sendmsg() calls. Thus tcp_bpf_send_verdict() keeps the verdict in
> psock->eval and runs the prog only when psock->eval is __SK_NONE. The code
> that resets psock->eval is after the switch statement, but the __SK_DROP
> branch returns before that code. Then sendmsg() drops all subsequent
> messages with -EACCES and does not run the prog.
>
> Reset psock->eval when apply_bytes is 0, as the __SK_REDIRECT branch does.
>
> Fixes: 4f738adba30a ("bpf: create tcp_bpf_ulp allowing BPF to monitor socket TX/RX data")
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>


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


> ---
>  net/ipv4/tcp_bpf.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index 8cf63a9e7fed..d90bcc911b36 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
> @@ -509,6 +509,9 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
>  	default:
>  		sk_msg_free(sk, msg);
>  		sk_msg_apply_bytes(psock, tosend);
> +		/* The verdict holds for apply_bytes, not for ever */
> +		if (!psock->apply_bytes)
> +			psock->eval = __SK_NONE;
>  		*copied -= (tosend + delta);
>  		return -EACCES;
>  	}

Re: [PATCH bpf v2 04/15] bpf, sockmap: Subtract all dropped bytes from the return value

[Jiayuan Chen]

On 11/2/26 3:21 PM, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
>
> When an sk_msg prog returns SK_DROP, tcp_bpf_send_verdict() frees the full
> message. It then subtracts tosend bytes from *copied, the return value of
> sendmsg(). But tosend is the number of bytes that the verdict applies to.
> After bpf_msg_apply_bytes() tosend can be less than the message size. Then
> sendmsg() reports dropped bytes as sent.
>
> Subtract the number of bytes that sk_msg_free() frees. After an error the
> SK_PASS branch already does this. The SK_REDIRECT branch does the same if
> the message is not the cork. Nothing changes when tosend is equal to the
> message size. Otherwise sendmsg() now returns -EACCES if it sends nothing.
>
> Fixes: ca70b8baf2bd ("tcp_bpf: Fix the sk_mem_uncharge logic in tcp_bpf_sendmsg")
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>


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


> ---
>  net/ipv4/tcp_bpf.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index b016aa419fb2..36d1bf46353a 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
> @@ -521,12 +521,12 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
>  		break;
>  	case __SK_DROP:
>  	default:
> -		sk_msg_free(sk, msg);
> +		/* All of it is thrown away, not only tosend bytes */
> +		*copied -= sk_msg_free(sk, msg) + delta;
>  		sk_msg_apply_bytes(psock, tosend);
>  		/* The verdict holds for apply_bytes, not for ever */
>  		if (!psock->apply_bytes)
>  			psock->eval = __SK_NONE;
> -		*copied -= (tosend + delta);
>  		return -EACCES;
>  	}
>  

Re: [PATCH bpf v2 05/15] bpf, sockmap: Clear the resume state of a stopped psock

[Jiayuan Chen]

On 11/2/26 3:21 PM, Alexei Starovoitov wrote:
> From: Josef Bacik <josef@toxicpanda.com>
>
> sk_psock_backlog() passes the skbs of psock->ingress_skb to the socket of
> the psock. When the socket has no room, the function saves the length and
> offset of the remaining data in psock->work_state. A later run continues at
> this offset and clears the state when the skb is complete.
>
> sk_psock_skb_state() saves and clears the state, but only while
> SK_PSOCK_TX_ENABLED is set. sk_psock_backlog() does not test this flag for
> each skb. If the socket closes during the loop, sk_psock_stop() clears the
> flag. The loop continues, but the state of the completed skb stays. The
> loop uses this state for all subsequent skbs and sends only part of their
> data. A closing socket may lose the end of a stream, but not the middle.
>
> The state belongs to the skb at the head of ingress_skb. Always clear the
> state when this skb is complete. A stopped psock still saves no new state.
>
> Fixes: 3b4f14b79428 ("bpf, sockmap: fix duplicated data transmission")
> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  net/core/skmsg.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/skmsg.c b/net/core/skmsg.c
> index f03519468ad1..a5d498982848 100644
> --- a/net/core/skmsg.c
> +++ b/net/core/skmsg.c
> @@ -664,7 +664,8 @@ static void sk_psock_skb_state(struct sk_psock *psock,
>  			       int len, int off)
>  {
>  	spin_lock_bh(&psock->ingress_lock);
> -	if (sk_psock_test_state(psock, SK_PSOCK_TX_ENABLED)) {
> +	/* No new state once the psock is stopped, but never keep an old one */
> +	if (!len || sk_psock_test_state(psock, SK_PSOCK_TX_ENABLED)) {
>  		state->len = len;
>  		state->off = off;
>  	}


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


The 'state' should be clear unconditionally.

Re: [PATCH bpf v2 06/15] bpf, sockmap: Give no progs to a TCP listener

[Jiayuan Chen]

On 11/2/26 3:21 PM, Alexei Starovoitov wrote:
> From: Chris Mason <mason@kernel.org>
>
> A psock with a verdict prog replaces sk_data_ready and sk_write_space of
> its socket. It saves the old callbacks, and the new callbacks call them.
> sock_map_link() does this to a TCP listener too. Its new sk_data_ready
> cannot read and does not call the old one. Thus it does not wake accept().
>
> sk_clone() copies the new callbacks to each child, but not the psock.
> tcp_bpf_clone() restores only sk_prot. Without a psock the child loses data
> or does not wake its reader. If it gets a psock, the psock saves the copies
> as the old callbacks. Then sk_psock_verdict_data_ready() calls itself until
> the stack overflows. sk_psock_write_space() calls itself too, without end.
>
> Give no progs to a TCP listener, as before the first commit below. A
> listener receives and sends no data. It stays in the map and keeps its own
> callbacks. Add this exception to map_sockmap.rst. A listener no longer gets
> -EBUSY from a second map with progs of the same kind.
>
> Since the second commit below, a socket keeps its psock when TCP unhashes
> it. sock_map_link() does not see the state that the socket gets later. Do
> not depend on TCP to keep such a socket out of TCP_LISTEN. Set both
> callbacks in tcp_bpf_clone() to the TCP defaults.
>
> Fixes: 0c48eefae712 ("sock_map: Lift socket state restriction for datagram sockets")
> Fixes: b8b8315e39ff ("bpf, sockmap: Remove unhash handler for BPF sockmap usage")
> Signed-off-by: Chris Mason <mason@kernel.org>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
> ---
>  Documentation/bpf/map_sockmap.rst |  4 +++-
>  net/core/sock_map.c               |  9 +++++++++
>  net/ipv4/tcp_bpf.c                | 15 +++++++++++++--
>  3 files changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/bpf/map_sockmap.rst b/Documentation/bpf/map_sockmap.rst
> index 2d630686a00b..eb466b86df17 100644
> --- a/Documentation/bpf/map_sockmap.rst
> +++ b/Documentation/bpf/map_sockmap.rst
> @@ -33,7 +33,9 @@ of ``__SK_DROP``, ``__SK_PASS``, or ``__SK_REDIRECT``.
>  
>  When a socket is inserted into one of these maps, its socket callbacks are
>  replaced and a ``struct sk_psock`` is attached to it. Additionally, this
> -``sk_psock`` inherits the programs that are attached to the map.
> +``sk_psock`` inherits the programs that are attached to the map. A TCP listener
> +is an exception. It gets a ``struct sk_psock``, but it keeps its callbacks and
> +inherits no programs.
>  
>  A sock object may be in multiple maps, but can only inherit a single
>  parse or verdict program. If adding a sock object to a map would result
> diff --git a/net/core/sock_map.c b/net/core/sock_map.c
> index efe49551caba..282c6a4b2b05 100644
> --- a/net/core/sock_map.c
> +++ b/net/core/sock_map.c
> @@ -228,6 +228,14 @@ static int sock_map_link(struct bpf_map *map, struct sock *sk)
>  	struct sk_psock *psock;
>  	int ret;
>  
> +	/*
> +	 * A listener receives no data. With the callbacks of a verdict program
> +	 * or a parser it would no longer wake accept(), and its children would
> +	 * copy them.
> +	 */
> +	if (sk_is_tcp(sk) && sk->sk_state == TCP_LISTEN)
> +		goto no_progs;
> +
>  	stream_verdict = READ_ONCE(progs->stream_verdict);
>  	if (stream_verdict) {
>  		stream_verdict = bpf_prog_inc_not_zero(stream_verdict);
> @@ -262,6 +270,7 @@ static int sock_map_link(struct bpf_map *map, struct sock *sk)
>  		}
>  	}
>  
> +no_progs:
>  	psock = sock_map_psock_get_checked(sk);
>  	if (IS_ERR(psock)) {
>  		ret = PTR_ERR(psock);
> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index 36d1bf46353a..fd443350f9b8 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
> @@ -785,7 +785,18 @@ void tcp_bpf_clone(const struct sock *sk, struct sock *newsk)
>  {
>  	struct proto *prot = newsk->sk_prot;
>  
> -	if (is_insidevar(prot, tcp_bpf_prots))
> -		newsk->sk_prot = sk->sk_prot_creator;
> +	if (!is_insidevar(prot, tcp_bpf_prots))
> +		return;
> +
> +	newsk->sk_prot = sk->sk_prot_creator;
> +	/*
> +	 * sock_map_link() gives a listener no programs, but a socket which got
> +	 * them in another state and listens now has the psock's sk_data_ready
> +	 * and sk_write_space, and sock_copy() has copied them.
> +	 * sk_psock_init() refuses a socket that has sk_user_data or a ULP, so
> +	 * what the psock replaced were the defaults of a TCP socket.
> +	 */
> +	newsk->sk_data_ready = sock_def_readable;
> +	newsk->sk_write_space = sk_stream_write_space;




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


I guess the scenario is: connect(AF_UNSPEC) unhashes a connected socket
that is still in the map, then listen() turns it into a listener. Since
we no longer have an unhash handler to drop the psock, the callbacks have
to be reset in tcp_bpf_clone().

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

[Jiayuan Chen]

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

Re: [PATCH bpf v2 08/15] net: Leave sk_stream_wait_memory() once the socket is dead

[Jiayuan Chen]

On 11/2/26 3:21 PM, Alexei Starovoitov wrote:
> From: Alexei Starovoitov <ast@kernel.org>
>
> When an sk_msg prog redirects data to the egress of another socket, the
> sender calls tcp_sendmsg_locked() on that socket. The sender holds a
> reference to the sock, but not to its file. Thus __tcp_close() can call
> sock_orphan() while the sender sleeps in sk_stream_wait_memory().
> sock_orphan() sets SOCK_DEAD and sets sk_wq to NULL.
>
> After the sleep the loop continues and does not test SOCK_DEAD. The
> subsequent sk_set_bit() can dereference the NULL sk_wq. The tests of sk_err
> and sk_shutdown do not always stop the sender. sock_error() clears sk_err,
> and __tcp_close() can call tcp_disconnect(), which clears sk_shutdown. The
> sender then adds data to the write queue of the dead socket, and
> inet_sock_destruct() warns.
>
> Leave the loop with -EPIPE when the socket is dead after the sleep. The
> function already tests SOCK_DEAD before remove_wait_queue() for the same
> reason. A caller that has the socket open never sees SOCK_DEAD.
>
> 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/core/stream.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/stream.c b/net/core/stream.c
> index cb255e2a3a5d..334124d683a7 100644
> --- a/net/core/stream.c
> +++ b/net/core/stream.c
> @@ -150,7 +150,11 @@ int sk_stream_wait_memory(struct sock *sk, long *timeo_p)
>  				    (sk_stream_memory_free(sk) && !vm_wait),
>  				    &wait);
>  		sk->sk_write_pending--;
> -		if (ret < 0)
> +		/*
> +		 * A sender which sockmap has redirected to sk does not keep it
> +		 * open. A dead socket has no sk_wq.
> +		 */
> +		if (ret < 0 || sock_flag(sk, SOCK_DEAD))
>  			goto do_error;
>  
>  		if (vm_wait) {

Re: [PATCH bpf v2 09/15] bpf, sockmap: Check the TCP state in recvmsg()

[Jiayuan Chen]

On 11/2/26 3:21 PM, Alexei Starovoitov wrote:
> From: Chris Mason <mason@kernel.org>
>
> With an sk_skb verdict prog, tcp_bpf_recvmsg_parser() copies data from the
> psock. Then it calls __tcp_cleanup_rbuf() to calculate the receive window
> and to send an ACK. The window and the ACK need a connection on the socket.
> But tcp_disconnect() keeps the socket in the sockmap and the data on the
> psock. For a TCP socket sock_map_redirect_allowed() rejects only
> TCP_LISTEN. Thus the psock can also get data after tcp_disconnect().
>
> __tcp_select_window() divides by icsk_ack.rcv_mss, which can be 0 in
> TCP_CLOSE. In TCP_SYN_SENT the new peer can get an ACK with the sequence
> numbers of the old connection.
>
> Call __tcp_cleanup_rbuf() only when tcp_can_send_ack() returns true.
> recvmsg() returns the data as before. Move tcp_can_send_ack() from MPTCP to
> tcp.h. It also returns false for a fastopen child in TCP_SYN_RECV. From its
> peer the child has only the SYN data, and the SYN-ACK acknowledges it.
>
> Fixes: e5c6de5fa025 ("bpf, sockmap: Incorrectly handling copied_seq")
> Signed-off-by: Chris Mason <mason@kernel.org>
> Signed-off-by: Alexei Starovoitov <ast@kernel.org>


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


> ---
>  include/net/tcp.h    | 6 ++++++
>  net/ipv4/tcp_bpf.c   | 3 ++-
>  net/mptcp/protocol.c | 6 ------
>  3 files changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/include/net/tcp.h b/include/net/tcp.h
> index 3560adddf821..23a22d70718e 100644
> --- a/include/net/tcp.h
> +++ b/include/net/tcp.h
> @@ -1845,6 +1845,12 @@ static inline void tcp_adjust_rcv_ssthresh(struct sock *sk)
>  void tcp_cleanup_rbuf(struct sock *sk, int copied);
>  void __tcp_cleanup_rbuf(struct sock *sk, int copied);
>  
> +/* Is the socket in a state in which TCP may send an ACK? */
> +static inline bool tcp_can_send_ack(const struct sock *sk)
> +{
> +	return !((1 << inet_sk_state_load(sk)) &
> +	       (TCPF_SYN_SENT | TCPF_SYN_RECV | TCPF_TIME_WAIT | TCPF_CLOSE | TCPF_LISTEN));
> +}
>  
>  /* We provision sk_rcvbuf around 200% of sk_rcvlowat.
>   * If 87.5 % (7/8) of the space has been consumed, we want to override
> diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
> index ef2e15de0fe3..3d703f1517d8 100644
> --- a/net/ipv4/tcp_bpf.c
> +++ b/net/ipv4/tcp_bpf.c
> @@ -343,7 +343,8 @@ static int tcp_bpf_recvmsg_parser(struct sock *sk,
>  	if (!peek)
>  		WRITE_ONCE(tcp->copied_seq, seq);
>  	tcp_rcv_space_adjust(sk);
> -	if (copied > 0)
> +	/* Data on the psock outlives the connection that it came in on */
> +	if (copied > 0 && tcp_can_send_ack(sk))
>  		__tcp_cleanup_rbuf(sk, copied);
>  
>  unlock:
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 7a22e9388e0e..d62cfbf2d490 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -619,12 +619,6 @@ static void mptcp_set_timeout(struct sock *sk)
>  	__mptcp_set_timeout(sk, tout);
>  }
>  
> -static inline bool tcp_can_send_ack(const struct sock *ssk)
> -{
> -	return !((1 << inet_sk_state_load(ssk)) &
> -	       (TCPF_SYN_SENT | TCPF_SYN_RECV | TCPF_TIME_WAIT | TCPF_CLOSE | TCPF_LISTEN));
> -}
> -
>  void __mptcp_subflow_send_ack(struct sock *ssk)
>  {
>  	if (tcp_can_send_ack(ssk))

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

[Alexei Starovoitov]

On Mon, Nov 02, 2026 at 04:32 AM Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
> 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

Thanks for testing.
All patches have reproducers.
I didn't share them, hough it's a weak defense.
Anyone can build a repro from the patch.

Re: [PATCH bpf v2 09/15] bpf, sockmap: Check the TCP state in recvmsg()

[Matthieu Baerts]

Hi Alexei, Chris,

Thank you for the fix!

On 02/11/2026 08:21, Alexei Starovoitov wrote:
> From: Chris Mason <mason@kernel.org>
> 
> With an sk_skb verdict prog, tcp_bpf_recvmsg_parser() copies data from the
> psock. Then it calls __tcp_cleanup_rbuf() to calculate the receive window
> and to send an ACK. The window and the ACK need a connection on the socket.
> But tcp_disconnect() keeps the socket in the sockmap and the data on the
> psock. For a TCP socket sock_map_redirect_allowed() rejects only
> TCP_LISTEN. Thus the psock can also get data after tcp_disconnect().
> 
> __tcp_select_window() divides by icsk_ack.rcv_mss, which can be 0 in
> TCP_CLOSE. In TCP_SYN_SENT the new peer can get an ACK with the sequence
> numbers of the old connection.
> 
> Call __tcp_cleanup_rbuf() only when tcp_can_send_ack() returns true.
> recvmsg() returns the data as before. Move tcp_can_send_ack() from MPTCP to
> tcp.h.

Good idea!

Acked-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH bpf v2 02/15] bpf, sockmap: Don't let an SK_DROP verdict stick

[John Fastabend]

On Mon, 02 Nov 2026 00:20, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>From: Josef Bacik <josef@toxicpanda.com>
>
>The verdict of an sk_msg prog is valid for one message, or for the bytes
>that bpf_msg_apply_bytes() requests. These bytes can come from many
>sendmsg() calls. Thus tcp_bpf_send_verdict() keeps the verdict in
>psock->eval and runs the prog only when psock->eval is __SK_NONE. The code
>that resets psock->eval is after the switch statement, but the __SK_DROP
>branch returns before that code. Then sendmsg() drops all subsequent
>messages with -EACCES and does not run the prog.
>
>Reset psock->eval when apply_bytes is 0, as the __SK_REDIRECT branch does.

So no one has tried to do apply_bytes + DROP I guess. If you want after
this series is merged I can add some test.

Thanks.

Reviewed-by: John Fastabend <john.fastabend@gmail.com>

>
>Fixes: 4f738adba30a ("bpf: create tcp_bpf_ulp allowing BPF to monitor socket TX/RX data")
>Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>---
> net/ipv4/tcp_bpf.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
>index 8cf63a9e7fed..d90bcc911b36 100644
>--- a/net/ipv4/tcp_bpf.c
>+++ b/net/ipv4/tcp_bpf.c
>@@ -509,6 +509,9 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
> 	default:
> 		sk_msg_free(sk, msg);
> 		sk_msg_apply_bytes(psock, tosend);
>+		/* The verdict holds for apply_bytes, not for ever */
>+		if (!psock->apply_bytes)
>+			psock->eval = __SK_NONE;
> 		*copied -= (tosend + delta);
> 		return -EACCES;
> 	}
>-- 
>2.55.0

Re: [PATCH bpf v2 03/15] bpf, sockmap: Don't let an SK_PASS verdict cover the next message

[John Fastabend]

On Mon, 02 Nov 2026 00:21, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>From: Alexei Starovoitov <ast@kernel.org>
>
>tcp_bpf_send_verdict() keeps the verdict of the sk_msg prog in psock->eval
>and runs the prog only when eval is __SK_NONE. For __SK_PASS it resets eval
>only after tcp_bpf_push() returns 0. But the push can release the socket
>lock in sk_stream_wait_memory(), and it can fail. Then another
>tcp_bpf_sendmsg() sends its message and does not run the prog.
>
>With bpf_msg_apply_bytes() the verdict covers apply_bytes bytes. After a
>failed push, apply_bytes still includes the sent bytes. Thus the verdict
>covers more bytes than the prog requested.
>
>Reset eval before the push when apply_bytes is 0. If this push fails, the
>prog now runs for the rest of the message. The same already occurs when a
>message does not fit into one sk_msg.
>
>When the push fails, subtract the sent bytes from apply_bytes. When
>apply_bytes is not 0, eval stays __SK_PASS during the push, and the prog
>does not run for another message, as today. The verdict must still cover
>the bytes that a failed push does not send. But such a message also lowers
>apply_bytes. Thus, if apply_bytes is 0 after the subtraction, end the
>verdict, as after a push that returns 0.

Reviewed-by: John Fastabend <john.fastabend@gmail.com>

>
>Fixes: 4f738adba30a ("bpf: create tcp_bpf_ulp allowing BPF to monitor socket TX/RX data")
>Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>---
> net/ipv4/tcp_bpf.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
>diff --git a/net/ipv4/tcp_bpf.c b/net/ipv4/tcp_bpf.c
>index d90bcc911b36..b016aa419fb2 100644
>--- a/net/ipv4/tcp_bpf.c
>+++ b/net/ipv4/tcp_bpf.c
>@@ -457,8 +457,22 @@ static int tcp_bpf_send_verdict(struct sock *sk, struct sk_psock *psock,
> 
> 	switch (psock->eval) {
> 	case __SK_PASS:
>+		/* The push may sleep without the socket lock, and it may fail */
>+		if (!psock->apply_bytes)
>+			psock->eval = __SK_NONE;
>+		origsize = msg->sg.size;
> 		ret = tcp_bpf_push(sk, msg, tosend, flags, true);
> 		if (unlikely(ret)) {
>+			/* A part of it may have gone out */
>+			sk_msg_apply_bytes(psock, origsize - msg->sg.size);
>+			/* Another sender can use up the rest during the push */

Interesting. We've never hit this with http parsers. We do use this to
skip payloads some times.

Thanks.

Re: [PATCH bpf v2 04/15] bpf, sockmap: Subtract all dropped bytes from the return value

[John Fastabend]

On Mon, 02 Nov 2026 00:21, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>From: Alexei Starovoitov <ast@kernel.org>
>
>When an sk_msg prog returns SK_DROP, tcp_bpf_send_verdict() frees the full
>message. It then subtracts tosend bytes from *copied, the return value of
>sendmsg(). But tosend is the number of bytes that the verdict applies to.
>After bpf_msg_apply_bytes() tosend can be less than the message size. Then
>sendmsg() reports dropped bytes as sent.
>
>Subtract the number of bytes that sk_msg_free() frees. After an error the
>SK_PASS branch already does this. The SK_REDIRECT branch does the same if
>the message is not the cork. Nothing changes when tosend is equal to the
>message size. Otherwise sendmsg() now returns -EACCES if it sends nothing.
>
>Fixes: ca70b8baf2bd ("tcp_bpf: Fix the sk_mem_uncharge logic in tcp_bpf_sendmsg")
>Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>---

Reviewed-by: John Fastabend <john.fastabend@gmail.com>

Re: [PATCH bpf v2 05/15] bpf, sockmap: Clear the resume state of a stopped psock

[John Fastabend]

On Mon, 02 Nov 2026 04:40, Jiayuan Chen <jiayuan.chen@linux.dev> wrote:
>
>On 11/2/26 3:21 PM, Alexei Starovoitov wrote:
>> From: Josef Bacik <josef@toxicpanda.com>
>>
>> sk_psock_backlog() passes the skbs of psock->ingress_skb to the socket of
>> the psock. When the socket has no room, the function saves the length and
>> offset of the remaining data in psock->work_state. A later run continues at
>> this offset and clears the state when the skb is complete.
>>
>> sk_psock_skb_state() saves and clears the state, but only while
>> SK_PSOCK_TX_ENABLED is set. sk_psock_backlog() does not test this flag for
>> each skb. If the socket closes during the loop, sk_psock_stop() clears the
>> flag. The loop continues, but the state of the completed skb stays. The
>> loop uses this state for all subsequent skbs and sends only part of their
>> data. A closing socket may lose the end of a stream, but not the middle.
>>
>> The state belongs to the skb at the head of ingress_skb. Always clear the
>> state when this skb is complete. A stopped psock still saves no new state.
>>
>> Fixes: 3b4f14b79428 ("bpf, sockmap: fix duplicated data transmission")
>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>> Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>> ---
>>  net/core/skmsg.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/net/core/skmsg.c b/net/core/skmsg.c
>> index f03519468ad1..a5d498982848 100644
>> --- a/net/core/skmsg.c
>> +++ b/net/core/skmsg.c
>> @@ -664,7 +664,8 @@ static void sk_psock_skb_state(struct sk_psock *psock,
>>  			       int len, int off)
>>  {
>>  	spin_lock_bh(&psock->ingress_lock);
>> -	if (sk_psock_test_state(psock, SK_PSOCK_TX_ENABLED)) {
>> +	/* No new state once the psock is stopped, but never keep an old one */
>> +	if (!len || sk_psock_test_state(psock, SK_PSOCK_TX_ENABLED)) {
>>  		state->len = len;
>>  		state->off = off;
>>  	}
>
>
>Reviewed-by: Jiayuan Chen <jiayuan.chen@linux.dev>

Reviewed-by: John Fastabend <john.fastabend@gmail.com>

Re: [PATCH bpf v2 06/15] bpf, sockmap: Give no progs to a TCP listener

[John Fastabend]

On Mon, 02 Nov 2026 00:21, Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
>From: Chris Mason <mason@kernel.org>
>
>A psock with a verdict prog replaces sk_data_ready and sk_write_space of
>its socket. It saves the old callbacks, and the new callbacks call them.
>sock_map_link() does this to a TCP listener too. Its new sk_data_ready
>cannot read and does not call the old one. Thus it does not wake accept().
>
>sk_clone() copies the new callbacks to each child, but not the psock.
>tcp_bpf_clone() restores only sk_prot. Without a psock the child loses data
>or does not wake its reader. If it gets a psock, the psock saves the copies
>as the old callbacks. Then sk_psock_verdict_data_ready() calls itself until
>the stack overflows. sk_psock_write_space() calls itself too, without end.
>
>Give no progs to a TCP listener, as before the first commit below. A
>listener receives and sends no data. It stays in the map and keeps its own
>callbacks. Add this exception to map_sockmap.rst. A listener no longer gets
>-EBUSY from a second map with progs of the same kind.
>
>Since the second commit below, a socket keeps its psock when TCP unhashes
>it. sock_map_link() does not see the state that the socket gets later. Do
>not depend on TCP to keep such a socket out of TCP_LISTEN. Set both
>callbacks in tcp_bpf_clone() to the TCP defaults.
>
>Fixes: 0c48eefae712 ("sock_map: Lift socket state restriction for datagram sockets")
>Fixes: b8b8315e39ff ("bpf, sockmap: Remove unhash handler for BPF sockmap usage")
>Signed-off-by: Chris Mason <mason@kernel.org>
>Signed-off-by: Alexei Starovoitov <ast@kernel.org>

Reviewed-by: John Fastabend <john.fastabend@gmail.com>

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

[John Fastabend]

On Mon, 02 Nov 2026 00:21, Alexei Starovoitov <alexei.starovoitov@gmail.com> 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.
>

Reviewed-by: John Fastabend <john.fastabend@gmail.com>

>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>
>---

Re: [PATCH bpf v2 00/15] bpf, sockmap: Fixes for verdicts, redirects, corks and recvmsg()

[John Fastabend]

On Mon, Nov 02, 2026 at 07:20:57AM +0000, Alexei Starovoitov wrote:
>From: Alexei Starovoitov <ast@kernel.org>
>
>These patches fix bugs in sockmap:
>
>  patches  area
>  -------  ----------------------------------------------------------------
>  01       recvmsg() after a fault
>  02-04    the verdict of an sk_msg prog and the return value of sendmsg()
>  05       the backlog of a stopped psock
>  06       TCP listeners
>  07-08    the target of an egress redirect
>  09       the TCP state in recvmsg()
>  10-14    corks and the redirect target
>  15       bpf_msg_push_data() and bpf_msg_pop_data()
>
>8 of the patches are from Chris Mason and Josef Bacik.

Thanks everyone lgtm. For the series.

Reviewed-by: John Fastabend <john.fastabend@gmail.com>

Re: [PATCH bpf v2 03/15] bpf, sockmap: Don't let an SK_PASS verdict cover the next message

[Alexei Starovoitov]

On Mon, Nov 02, 2026 at 10:51 AM John Fastabend <john.fastabend@gmail.com> wrote:
>>+			/* A part of it may have gone out */
>>+			sk_msg_apply_bytes(psock, origsize - msg->sg.size);
>>+			/* Another sender can use up the rest during the push */
>
> Interesting. We've never hit this with http parsers. We do use this to
> skip payloads some times.

That one needs two senders on one socket.
With one sender per socket you can't hit it.
A non-blocking send() fills sndbuf in the middle of the push.
N bytes go out and tcp_bpf_push() returns -EAGAIN.
Today apply_bytes still includes these N bytes.
Then the prog doesn't see the first N bytes after the payload.
There is no error and no splat, so it's easy to miss.