[PATCH net-next v2 0/4] net: psp: require an established connection for association setup

[PATCH net-next v2 0/4] net: psp: require an established connection for association setup

[Daniel Zahka]

This series removes support for using PSP's assoc uapi on TCP sockets
not in established state.

The PSP uapi and connection upgrade described in psp.rst is only fleshed
out for established TCP connections. Installing PSP assoc state on a
listen socket, or closed socket ahead of connect() is possible, but not
something that results in useful outcomes.

With commit 8cc3aef0cb19 ("tcp: Do not allow buggy transitions between
ehash and lhash2.") in place, this series makes it impossible to have
PSP assoc state on a listen socket. It is still possible to have a
closed socket with assoc state by terminating a connection with
shutdown(), but the series then adds code to prevent subsequent calls to
connect() from succeeding. Patch two updates psp.rst to discuss what
this means for users.

The first patch converts some tests that used TCP_CLOSE sockets for
basic uapi tests with connected sockets, so that the subsequent commit
doesn't break them.

The second patch introduces the actual checks on sk->sk_state during the
rx and tx assoc handlers, as well as preventing connect() attempts when
assoc state is already present. The commit message contains my argument
for why removing these "features" is appropriate and doesn't constitute
a fix.

The third patch unwinds commit 1d2929d0850f ("net: psp: do not inherit
the Rx association on clone"), which was introduced to workaround assoc
state not being handled correctly from listen sockets.

The fourth patch has some tests for the new checks introduced.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
Changes in v2:
- net: psp: require an established connection for association setup
  - reject connect() on sockets with PSP assoc state
  - drop PSP MSS overhead handling from tcp_v{4,6}_connect()
  - update psp.rst disconnect paragraph
- selftests: drv-net: psp: test that assocs require an established socket
  - drop assoc_tx_non_established test
- Link to v1: https://lore.kernel.org/r/20260925-psp-defeat-v1-0-9f0b430107aa@gmail.com

---
Daniel Zahka (4):
      selftests: drv-net: psp: swap closed for connected sockets in assoc tests
      net: psp: require an established connection for association setup
      net: psp: drop psp assoc clear in sk_clone()
      selftests: drv-net: psp: test that rx-assoc fails on closed and listen socks

 Documentation/networking/psp.rst           |  8 +++++
 net/core/sock.c                            |  2 +-
 net/ipv4/tcp_ipv4.c                        |  7 ++--
 net/ipv6/tcp_ipv6.c                        |  9 +++--
 net/psp/psp_sock.c                         | 12 +++++++
 tools/testing/selftests/drivers/net/psp.py | 56 ++++++++++++++++++++++++------
 6 files changed, 77 insertions(+), 17 deletions(-)
---
base-commit: 1631d79ae57dce2c5f88ad278307028638a8b4d9
change-id: 20260916-psp-defeat-a271649be9dc

Best regards,
-- 
Daniel Zahka <daniel.zahka@gmail.com>

[PATCH net-next v2 1/4] selftests: drv-net: psp: swap closed for connected sockets in assoc tests

[Daniel Zahka]

Future work will only allow rx-assoc and tx-assoc to be performed when
the sock is in TCP_ESTABLISHED state.

Several assoc_ tests, as well as dev_rotate_spi, test using the rx-assoc
and tx-assoc uapi calls against sockets in TCP_CLOSE state.
These tests don't involve sending or receiving data, nor involve looking
up psp device by dst entry, so using a disposable disconnected socket
was just a convenience. These can be replaced by a disposable loopback
socket.

The loopback sockets are IPv4, where the sockets they replace were
AF_INET6. This doesn't reduce coverage, since the assoc handlers accept
any TCP socket and don't look at the address family.

Some users of rx-assoc and tx-assoc on closed sockets are left if they
validate errors that are returned before the kernel will check the
socket for TCP_ESTABLISHED.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
 tools/testing/selftests/drivers/net/psp.py | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
index 5a81f40cac7d..0a2329f41431 100755
--- a/tools/testing/selftests/drivers/net/psp.py
+++ b/tools/testing/selftests/drivers/net/psp.py
@@ -11,6 +11,8 @@ import struct
 import termios
 import time
 
+from contextlib import contextmanager
+
 from lib.py import defer
 from lib.py import ksft_run, ksft_exit, ksft_pr
 from lib.py import ksft_true, ksft_eq, ksft_ne, ksft_gt, ksft_raises
@@ -58,6 +60,17 @@ def _make_psp_conn(cfg, version=0, ipver=None):
     return s
 
 
+@contextmanager
+def _make_lo_conn():
+    # After tx-assoc, the client's egress is dropped, since lo has no
+    # psp_dev, so its FIN never reaches the server. Closing the server
+    # resets the unaccepted child, and the client accepts the cleartext
+    # RST because it hasn't received any PSP traffic yet.
+    with socket.create_server(("localhost", 0)) as srv, \
+         socket.create_connection(srv.getsockname()[:2]) as s:
+        yield s
+
+
 def _close_conn(cfg, s):
     _send_with_ack(cfg, b'data close\0')
     s.close()
@@ -200,20 +213,18 @@ def dev_rotate_spi(cfg):
     _init_psp_dev(cfg)
 
     top_a = top_b = 0
-    with socket.socket(socket.AF_INET6, socket.SOCK_STREAM) as s:
+    with _make_lo_conn() as s:
         assoc_a = cfg.pspnl.rx_assoc({"version": 0,
                                      "dev-id": cfg.psp_dev_id,
                                      "sock-fd": s.fileno()})
         top_a = assoc_a['rx-key']['spi'] >> 31
-        s.close()
     rot = cfg.pspnl.key_rotate({"id": cfg.psp_dev_id})
-    with socket.socket(socket.AF_INET6, socket.SOCK_STREAM) as s:
+    with _make_lo_conn() as s:
         ksft_eq(rot['id'], cfg.psp_dev_id)
         assoc_b = cfg.pspnl.rx_assoc({"version": 0,
                                     "dev-id": cfg.psp_dev_id,
                                     "sock-fd": s.fileno()})
         top_b = assoc_b['rx-key']['spi'] >> 31
-        s.close()
     ksft_ne(top_a, top_b)
 
 
@@ -221,7 +232,7 @@ def assoc_basic(cfg):
     """ Test creating associations """
     _init_psp_dev(cfg)
 
-    with socket.socket(socket.AF_INET6, socket.SOCK_STREAM) as s:
+    with _make_lo_conn() as s:
         assoc = cfg.pspnl.rx_assoc({"version": 0,
                                   "dev-id": cfg.psp_dev_id,
                                   "sock-fd": s.fileno()})
@@ -234,7 +245,6 @@ def assoc_basic(cfg):
                                   "tx-key": assoc['rx-key'],
                                   "sock-fd": s.fileno()})
         ksft_eq(len(assoc), 0)
-        s.close()
 
 
 def assoc_bad_dev(cfg):
@@ -320,7 +330,7 @@ def assoc_version_mismatch(cfg):
     # Translate versions to integers
     versions = [cfg.pspnl.consts["version"].entries[v].value for v in versions]
 
-    with socket.socket(socket.AF_INET6, socket.SOCK_STREAM) as s:
+    with _make_lo_conn() as s:
         rx = cfg.pspnl.rx_assoc({"version": versions[0],
                                  "dev-id": cfg.psp_dev_id,
                                  "sock-fd": s.fileno()})
@@ -393,7 +403,7 @@ def assoc_twice(cfg):
 
         return assoc
 
-    with socket.socket(socket.AF_INET6, socket.SOCK_STREAM) as s:
+    with _make_lo_conn() as s:
         assoc = rx_assoc_check(s)
         tx = cfg.pspnl.tx_assoc({"dev-id": cfg.psp_dev_id,
                                "version": 0,
@@ -402,7 +412,7 @@ def assoc_twice(cfg):
         ksft_eq(len(tx), 0)
 
         # Use the same Tx assoc second time
-        with socket.socket(socket.AF_INET6, socket.SOCK_STREAM) as s2:
+        with _make_lo_conn() as s2:
             rx_assoc_check(s2)
             tx = cfg.pspnl.tx_assoc({"dev-id": cfg.psp_dev_id,
                                    "version": 0,
@@ -410,8 +420,6 @@ def assoc_twice(cfg):
                                    "sock-fd": s2.fileno()})
             ksft_eq(len(tx), 0)
 
-        s.close()
-
 
 def _data_basic_send(cfg, version, ipver):
     """ Test basic data send """

-- 
2.52.0

[PATCH net-next v2 2/4] net: psp: require an established connection for association setup

[Daniel Zahka]

Check sk_state under the socket lock in both the rx-assoc and tx-assoc
handlers, and only allow association setup on sockets in
TCP_ESTABLISHED. Also, fail connect() when PSP assoc state is already
present, and remove the dead PSP MSS adjustment from
tcp_v[46]_connect().

The net effect of this commit is:
1. PSP assoc state can never exist on a listen socket.
2. The upgrade to PSP must be done while the socket is in
   TCP_ESTABLISHED.

This change defeatures behavior that was previously allowed under the
PSP uapi. My justification is:

Nothing useful can be done after association setup on closed or listen
sockets today. Listen sockets could accept a PSP-encrypted TCP SYN, but
the child socket will not inherit any PSP state. On the other side,
establishing PSP state prior to connect() will result in a PSP-encrypted
TCP SYN sent to a listening peer, which in turn has the aforementioned
limitations. That implies that there cannot be any users of this
feature, so it should be safe to remove it from the PSP uapi.

In theory, the check in the tx-assoc path is more restrictive than
necessary. FIN_WAIT1/2, CLOSING, LAST_ACK and CLOSE_WAIT could be
allowed, and the peer would accept PSP-encrypted ACKs in the
post-FIN-sent states, or data in the half-close case, but it is simpler
to disallow those states because they don't fit the upgrade model.

The check in the tx-assoc path fixes a bug in commit 6b46ca260e22 ("net:
psp: add socket security association code") where an unsynchronized
write can be performed on an assoc shared with a timewait socket when
the socket is in TCP_CLOSE after shutdown. This commit is not targeted
at net because its premise of preventing listen sockets from holding
assoc state depends on net-next commit 8cc3aef0cb19 ("tcp: Do not allow
buggy transitions between ehash and lhash2.")

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
v2:
- reject connect() on sockets with PSP assoc state
- drop PSP MSS overhead handling from tcp_v{4,6}_connect()
---
 Documentation/networking/psp.rst |  8 ++++++++
 net/ipv4/tcp_ipv4.c              |  7 +++++--
 net/ipv6/tcp_ipv6.c              |  9 ++++++---
 net/psp/psp_sock.c               | 12 ++++++++++++
 4 files changed, 31 insertions(+), 5 deletions(-)

diff --git a/Documentation/networking/psp.rst b/Documentation/networking/psp.rst
index 4ac09e64e95a..0f9b6b73f244 100644
--- a/Documentation/networking/psp.rst
+++ b/Documentation/networking/psp.rst
@@ -132,6 +132,14 @@ numbers in a way that deletes a prefix of the PSP protected part of
 the TCP stream. If userspace cares to mitigate this type of attack, a
 special "start of PSP" message should be exchanged after ``tx-assoc``.
 
+Upgrade to PSP must be done on established TCP connections.
+``rx-assoc`` and ``tx-assoc`` will return ``-ENOTCONN`` if
+``sk_state`` is not ``TCP_ESTABLISHED``.
+
+The PSP assoc state of a socket is not reset when the connection is
+torn down. ``connect()`` on a socket that has PSP assoc state will
+return ``-EINVAL``.
+
 Rotation notifications
 ----------------------
 
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index c107b7e44e5b..410d63d9e5c1 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -238,6 +238,9 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr_unsized *uaddr, int addr_len
 	if (usin->sin_family != AF_INET)
 		return -EAFNOSUPPORT;
 
+	if (psp_sk_assoc(sk))
+		return -EINVAL;
+
 	nexthop = daddr = usin->sin_addr.s_addr;
 	inet_opt = rcu_dereference_protected(inet->inet_opt,
 					     lockdep_sock_is_held(sk));
@@ -291,9 +294,9 @@ int tcp_v4_connect(struct sock *sk, struct sockaddr_unsized *uaddr, int addr_len
 	inet->inet_dport = usin->sin_port;
 	sk_daddr_set(sk, daddr);
 
-	inet_csk(sk)->icsk_ext_hdr_len = psp_sk_overhead(sk);
+	inet_csk(sk)->icsk_ext_hdr_len = 0;
 	if (inet_opt)
-		inet_csk(sk)->icsk_ext_hdr_len += inet_opt->opt.optlen;
+		inet_csk(sk)->icsk_ext_hdr_len = inet_opt->opt.optlen;
 
 	tp->rx_opt.mss_clamp = TCP_MSS_DEFAULT;
 
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index afb4d8e9fa67..e006a30360ea 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -153,6 +153,9 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
 	if (usin->sin6_family != AF_INET6)
 		return -EAFNOSUPPORT;
 
+	if (psp_sk_assoc(sk))
+		return -EINVAL;
+
 	fl6 = &inet_sk(sk)->cork.fl.u.ip6;
 	memset(fl6, 0, sizeof(*fl6));
 
@@ -311,10 +314,10 @@ static int tcp_v6_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
 	sk->sk_gso_type = SKB_GSO_TCPV6;
 	ip6_dst_store(sk, dst, false, false);
 
-	icsk->icsk_ext_hdr_len = psp_sk_overhead(sk);
+	icsk->icsk_ext_hdr_len = 0;
 	if (opt)
-		icsk->icsk_ext_hdr_len += opt->opt_flen +
-					  opt->opt_nflen;
+		icsk->icsk_ext_hdr_len = opt->opt_flen +
+					 opt->opt_nflen;
 
 	tp->rx_opt.mss_clamp = IPV6_MIN_MTU - sizeof(struct tcphdr) - sizeof(struct ipv6hdr);
 
diff --git a/net/psp/psp_sock.c b/net/psp/psp_sock.c
index a9cfeebe4ba1..a6b1c42dd626 100644
--- a/net/psp/psp_sock.c
+++ b/net/psp/psp_sock.c
@@ -159,6 +159,12 @@ int psp_sock_assoc_set_rx(struct sock *sk, struct psp_assoc *pas,
 
 	lock_sock(sk);
 
+	if (sk->sk_state != TCP_ESTABLISHED) {
+		NL_SET_ERR_MSG(extack, "Socket must be in established state");
+		err = -ENOTCONN;
+		goto exit_unlock;
+	}
+
 	if (psp_sk_assoc(sk)) {
 		NL_SET_ERR_MSG(extack, "Socket already has PSP state");
 		err = -EBUSY;
@@ -252,6 +258,12 @@ int psp_sock_assoc_set_tx(struct sock *sk, struct psp_dev *psd,
 
 	lock_sock(sk);
 
+	if (sk->sk_state != TCP_ESTABLISHED) {
+		NL_SET_ERR_MSG(extack, "Socket must be in established state");
+		err = -ENOTCONN;
+		goto exit_unlock;
+	}
+
 	pas = psp_sk_assoc(sk);
 	if (!pas) {
 		NL_SET_ERR_MSG(extack, "Socket has no Rx key");

-- 
2.52.0

[PATCH net-next v2 3/4] net: psp: drop psp assoc clear in sk_clone()

[Daniel Zahka]

Unwind the change from commit 1d2929d0850f ("net: psp: do not inherit
the Rx association on clone"), now that assoc state cannot exist on
listen sockets.

With commit 8cc3aef0cb19 ("tcp: Do not allow buggy transitions between
ehash and lhash2."), and the change in this series restricting PSP assoc
operations to established sockets, this state should be unreachable.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
 net/core/sock.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/sock.c b/net/core/sock.c
index 2948dffcc3e1..d7ddd365abeb 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2496,7 +2496,7 @@ struct sock *sk_clone(const struct sock *sk, const gfp_t priority,
 	RCU_INIT_POINTER(newsk->sk_bpf_storage, NULL);
 #endif
 #if IS_ENABLED(CONFIG_INET_PSP)
-	RCU_INIT_POINTER(newsk->psp_assoc, NULL);
+	DEBUG_NET_WARN_ON_ONCE(rcu_access_pointer(sk->psp_assoc));
 #endif
 
 	/* SANITY */

-- 
2.52.0

[PATCH net-next v2 4/4] selftests: drv-net: psp: test that rx-assoc fails on closed and listen socks

[Daniel Zahka]

Cover two of the basic psp assoc states that have been defeatured. Do
not include tests with shutdown(), connect(..., AF_UNSPEC), etc. I don't
think the coverage is worth the complexity. That should be done with
packetdrill.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
v2:
- drop assoc_tx_non_established test
---
 tools/testing/selftests/drivers/net/psp.py | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
index 0a2329f41431..473500901879 100755
--- a/tools/testing/selftests/drivers/net/psp.py
+++ b/tools/testing/selftests/drivers/net/psp.py
@@ -319,6 +319,32 @@ def assoc_sk_only_unconn(cfg):
         ksft_eq(the_exception.nl_msg.error, -errno.EINVAL)
 
 
+def assoc_rx_unconnected(cfg):
+    """ Test that an Rx assoc is rejected on an unconnected socket """
+    _init_psp_dev(cfg)
+
+    with socket.socket(socket.AF_INET6, socket.SOCK_STREAM) as s:
+        with ksft_raises(NlError) as cm:
+            cfg.pspnl.rx_assoc({"version": 0,
+                                "dev-id": cfg.psp_dev_id,
+                                "sock-fd": s.fileno()})
+        ksft_eq(cm.exception.nl_msg.error, -errno.ENOTCONN)
+        ksft_eq(cm.exception.nl_msg.extack['bad-attr'], ".sock-fd")
+
+
+def assoc_rx_listener(cfg):
+    """ Test that an Rx assoc is rejected on a listening socket """
+    _init_psp_dev(cfg)
+
+    with socket.create_server(("localhost", 0)) as s:
+        with ksft_raises(NlError) as cm:
+            cfg.pspnl.rx_assoc({"version": 0,
+                                "dev-id": cfg.psp_dev_id,
+                                "sock-fd": s.fileno()})
+        ksft_eq(cm.exception.nl_msg.error, -errno.ENOTCONN)
+        ksft_eq(cm.exception.nl_msg.extack['bad-attr'], ".sock-fd")
+
+
 def assoc_version_mismatch(cfg):
     """ Test creating associations where Rx and Tx PSP versions do not match """
     _init_psp_dev(cfg)

-- 
2.52.0

Re: [PATCH net-next v2 0/4] net: psp: require an established connection for association setup

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net-next v2 0/4] net: psp: require an established connection for association setup

[Daniel Zahka]

On Wed Sep 30, 2026 at 7:48 PM EDT, netdev-bot+sinfo wrote:
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
>
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

This patch could be taken as a fix with:
Fixes: 6b46ca260e22 ("net: psp: add socket security association code")

The main caveat is that the premise of defeaturing PSP listen sockets
relies on two net-next commits:
commit 8cc3aef0cb19 ("tcp: Do not allow buggy transitions between ehash and lhash2.")
commit 404381b4f105 ("tcp: Set unhashed_state in inet_twsk_hashdance_schedule().")

without the above commits, the part of the commit message about
defeaturing listen sockets is not true. I considered having just this
patch go into net without the promise of defeaturing PSP on listen
sockets, but I thought that would be suspect, because a reviewer could
point out that we are jumping through hoops to restrict to establishd,
but the socket could then be disconnected and transitioned to listen.

Re: [PATCH net-next v2 1/4] selftests: drv-net: psp: swap closed for connected sockets in assoc tests

[Willem de Bruijn]

Daniel Zahka wrote:
> Future work will only allow rx-assoc and tx-assoc to be performed when
> the sock is in TCP_ESTABLISHED state.
> 
> Several assoc_ tests, as well as dev_rotate_spi, test using the rx-assoc
> and tx-assoc uapi calls against sockets in TCP_CLOSE state.
> These tests don't involve sending or receiving data, nor involve looking
> up psp device by dst entry, so using a disposable disconnected socket
> was just a convenience. These can be replaced by a disposable loopback
> socket.
> 
> The loopback sockets are IPv4, where the sockets they replace were
> AF_INET6. This doesn't reduce coverage, since the assoc handlers accept
> any TCP socket and don't look at the address family.
> 
> Some users of rx-assoc and tx-assoc on closed sockets are left if they
> validate errors that are returned before the kernel will check the
> socket for TCP_ESTABLISHED.
> 
> Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Re: [PATCH net-next v2 2/4] net: psp: require an established connection for association setup

[Willem de Bruijn]

Daniel Zahka wrote:
> Check sk_state under the socket lock in both the rx-assoc and tx-assoc
> handlers, and only allow association setup on sockets in
> TCP_ESTABLISHED. Also, fail connect() when PSP assoc state is already
> present, and remove the dead PSP MSS adjustment from
> tcp_v[46]_connect().
> 
> The net effect of this commit is:
> 1. PSP assoc state can never exist on a listen socket.
> 2. The upgrade to PSP must be done while the socket is in
>    TCP_ESTABLISHED.
> 
> This change defeatures behavior that was previously allowed under the
> PSP uapi. My justification is:
> 
> Nothing useful can be done after association setup on closed or listen
> sockets today. Listen sockets could accept a PSP-encrypted TCP SYN, but
> the child socket will not inherit any PSP state. On the other side,
> establishing PSP state prior to connect() will result in a PSP-encrypted
> TCP SYN sent to a listening peer, which in turn has the aforementioned
> limitations. That implies that there cannot be any users of this
> feature, so it should be safe to remove it from the PSP uapi.
> 
> In theory, the check in the tx-assoc path is more restrictive than
> necessary. FIN_WAIT1/2, CLOSING, LAST_ACK and CLOSE_WAIT could be
> allowed, and the peer would accept PSP-encrypted ACKs in the
> post-FIN-sent states, or data in the half-close case, but it is simpler
> to disallow those states because they don't fit the upgrade model.

And preferable to do so. While it could be allowed, there is no real
use case for it. Simpler state model allows simpler code.

> The check in the tx-assoc path fixes a bug in commit 6b46ca260e22 ("net:
> psp: add socket security association code") where an unsynchronized
> write can be performed on an assoc shared with a timewait socket when
> the socket is in TCP_CLOSE after shutdown. This commit is not targeted
> at net because its premise of preventing listen sockets from holding
> assoc state depends on net-next commit 8cc3aef0cb19 ("tcp: Do not allow
> buggy transitions between ehash and lhash2.")
> 
> Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Even if targeting to net-next, you could consider keeping the Fixes tag.
Importantly the prerequisite patch is mentioned. But is only one of two
mentioned here?

Re: [PATCH net-next v2 3/4] net: psp: drop psp assoc clear in sk_clone()

[Willem de Bruijn]

Daniel Zahka wrote:
> Unwind the change from commit 1d2929d0850f ("net: psp: do not inherit
> the Rx association on clone"), now that assoc state cannot exist on
> listen sockets.
> 
> With commit 8cc3aef0cb19 ("tcp: Do not allow buggy transitions between
> ehash and lhash2."), and the change in this series restricting PSP assoc
> operations to established sockets, this state should be unreachable.
> 
> Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Re: [PATCH net-next v2 4/4] selftests: drv-net: psp: test that rx-assoc fails on closed and listen socks

[Willem de Bruijn]

Daniel Zahka wrote:
> Cover two of the basic psp assoc states that have been defeatured. Do
> not include tests with shutdown(), connect(..., AF_UNSPEC), etc. I don't
> think the coverage is worth the complexity. That should be done with
> packetdrill.
> 
> Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>

Reviewed-by: Willem de Bruijn <willemb@google.com>

Re: [PATCH net-next v2 2/4] net: psp: require an established connection for association setup

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 0 · Medium: 1 · Low: 2

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The new check in psp_sock_assoc_set_rx()
  (net/psp/psp_sock.c:162) rejects every state other than…
- [Low] The commit message says the new sk_state check in
  psp_sock_assoc_set_tx() (net/psp/psp_sock.c:261) fixes a bug from…
- [Low] The new psp.rst text (lines 139-141) says "connect() on a socket
  that has PSP assoc state will return -EINVAL".

--

net: psp: require an established connection for association setup

This commit makes the PSP rx-assoc and tx-assoc handlers reject sockets
that are not in TCP_ESTABLISHED. It also makes tcp_v4_connect() and
tcp_v6_connect() return -EINVAL when PSP assoc state already exists, and
drops the PSP overhead from icsk_ext_hdr_len in the connect paths.

> diff --git a/Documentation/networking/psp.rst b/Documentation/networking/psp.rst
> index 4ac09e64e95a5..0f9b6b73f2447 100644
> --- a/Documentation/networking/psp.rst
> +++ b/Documentation/networking/psp.rst
> @@ -132,6 +132,14 @@ numbers in a way that deletes a prefix of the PSP protected part of
>  the TCP stream. If userspace cares to mitigate this type of attack, a
>  special "start of PSP" message should be exchanged after ``tx-assoc``.
>  
> +Upgrade to PSP must be done on established TCP connections.
> +``rx-assoc`` and ``tx-assoc`` will return ``-ENOTCONN`` if
> +``sk_state`` is not ``TCP_ESTABLISHED``.
> +
> +The PSP assoc state of a socket is not reset when the connection is
> +torn down. ``connect()`` on a socket that has PSP assoc state will
> +return ``-EINVAL``.

[Severity: Low]
Is the -EINVAL statement accurate for most connect() calls on a socket
with assoc state?

rx-assoc and tx-assoc now require TCP_ESTABLISHED, so a socket with
assoc state has sock->state == SS_CONNECTED. __inet_stream_connect()
returns before it gets to sk_prot->connect():

net/ipv4/af_inet.c:__inet_stream_connect() {
    ...
	switch (sock->state) {
	default:
		err = -EINVAL;
		goto out;
	case SS_CONNECTED:
		err = -EISCONN;
		goto out;
    ...
}

FIN or RST teardown leaves sock->state at SS_CONNECTED, and
psp_sk_assoc_free() only runs from inet_sock_destruct(). That means the
new psp_sk_assoc() checks in tcp_v4_connect() and tcp_v6_connect() can
only be reached after an explicit connect(AF_UNSPEC) disconnect. Every
other case returns -EISCONN.

Should the documentation say that -EINVAL applies to reconnect attempts
after an AF_UNSPEC disconnect?

> +
>  Rotation notifications
>  ----------------------
>  

[ ... ]

> diff --git a/net/psp/psp_sock.c b/net/psp/psp_sock.c
> index a9cfeebe4ba1b..a6b1c42dd626f 100644
> --- a/net/psp/psp_sock.c
> +++ b/net/psp/psp_sock.c
> @@ -159,6 +159,12 @@ int psp_sock_assoc_set_rx(struct sock *sk, struct psp_assoc *pas,
>  
>  	lock_sock(sk);
>  
> +	if (sk->sk_state != TCP_ESTABLISHED) {
> +		NL_SET_ERR_MSG(extack, "Socket must be in established state");
> +		err = -ENOTCONN;
> +		goto exit_unlock;
> +	}
> +

[Severity: Medium]
This check in psp_sock_assoc_set_rx() rejects every state except
TCP_ESTABLISHED. The commit message only justifies this with closed and
listen sockets. Does it also remove cases that could have users?

A TCP Fast Open server child returned by accept() can still be in
TCP_SYN_RECV. It already has a dst from tcp_v4_syn_recv_sock() or
tcp_v6_syn_recv_sock(), and it is the same socket that later becomes
ESTABLISHED. The point that the child does not inherit PSP state doesn't
apply to it. On that socket, rx-assoc now succeeds or fails depending on
when the final ACK arrives.

A TCP_FASTOPEN_CONNECT client also returns from connect() while it is
still in TCP_SYN_SENT.

The commit message also says:

  "establishing PSP state prior to connect() will result in a
   PSP-encrypted TCP SYN sent to a listening peer"

Is this true when only the Rx key is installed? The SYN is built by
tcp_init_nondata_skb(), which calls psp_enqueue_set_decrypted():

include/net/psp/functions.h:psp_enqueue_set_decrypted() {
	...
	pas = psp_sk_assoc(sk);
	if (pas && pas->tx.spi)
		skb->decrypted = 1;
}

With only an Rx key, the SYN goes out in plaintext.
__psp_sk_rx_policy_check() accepts plaintext as long as no Tx key is
set:

	if (!pse) {
		if (!pas->tx.spi ||
		    (!pas->peer_tx && psp_is_allowed_nondata(skb, pas)))
			return 0;
	}

So the handshake completes normally, and tx-assoc can follow once the
connection is ESTABLISHED. With an explicit dev-id,
psp_assoc_device_get_locked() allows rx-assoc on a socket that has not
called connect() yet.

Before this patch, setting up only the Rx key before connect() appeared
to work. Should the commit message and psp.rst say that this sequence
and the TFO cases are being removed as well?

>  	if (psp_sk_assoc(sk)) {
>  		NL_SET_ERR_MSG(extack, "Socket already has PSP state");
>  		err = -EBUSY;
> @@ -252,6 +258,12 @@ int psp_sock_assoc_set_tx(struct sock *sk, struct psp_dev *psd,
>  
>  	lock_sock(sk);
>  
> +	if (sk->sk_state != TCP_ESTABLISHED) {
> +		NL_SET_ERR_MSG(extack, "Socket must be in established state");
> +		err = -ENOTCONN;
> +		goto exit_unlock;
> +	}
> +

[Severity: Low]
The commit message says this check fixes a bug from commit 6b46ca260e22
("net: psp: add socket security association code"), but the patch has
no Fixes: tag. Could the tx-assoc check be split out and sent to net
with a Fixes: tag?

After tcp_time_wait(), psp_twsk_init() makes the timewait socket share
the same assoc:

net/psp/psp_sock.c:psp_twsk_init() {
	...
	if (pas)
		refcount_inc(&pas->refcnt);
	rcu_assign_pointer(tw->psp_assoc, pas);
	...
}

The original sk is then in TCP_CLOSE with sk->psp_assoc still set. If
only the Rx key was installed, the old psp_sock_assoc_set_tx() accepted
the request. psp_sock_set_tx_key() then wrote pas->tx, pas->upgrade_seq
and pas->drv_data. Meanwhile the timewait socket reads pas->tx.spi
without locks from softirq, in psp_twsk_rx_policy_check() and
psp_reply_set_decrypted().

The commit message says the patch targets net-next because it depends on
commit 8cc3aef0cb19 ("tcp: Do not allow buggy transitions between ehash
and lhash2."). That dependency only matters for the rx-assoc and listen
socket part. The tx-assoc state check works without it.

As posted, stable kernels with PSP would keep this race, and backporters
would have nothing pointing them to the fix.

>  	pas = psp_sk_assoc(sk);
>  	if (!pas) {
>  		NL_SET_ERR_MSG(extack, "Socket has no Rx key");

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260930-psp-defeat-v2-0-f266e7447129%40gmail.com

Re: [PATCH net-next v2 2/4] net: psp: require an established connection for association setup

[Daniel Zahka]

On Thu Oct 1, 2026 at 8:19 PM EDT, Willem de Bruijn wrote:
> Daniel Zahka wrote:
>> Check sk_state under the socket lock in both the rx-assoc and tx-assoc
>> handlers, and only allow association setup on sockets in
>> TCP_ESTABLISHED. Also, fail connect() when PSP assoc state is already
>> present, and remove the dead PSP MSS adjustment from
>> tcp_v[46]_connect().
>> 
>> The net effect of this commit is:
>> 1. PSP assoc state can never exist on a listen socket.
>> 2. The upgrade to PSP must be done while the socket is in
>>    TCP_ESTABLISHED.
>> 
>> This change defeatures behavior that was previously allowed under the
>> PSP uapi. My justification is:
>> 
>> Nothing useful can be done after association setup on closed or listen
>> sockets today. Listen sockets could accept a PSP-encrypted TCP SYN, but
>> the child socket will not inherit any PSP state. On the other side,
>> establishing PSP state prior to connect() will result in a PSP-encrypted
>> TCP SYN sent to a listening peer, which in turn has the aforementioned
>> limitations. That implies that there cannot be any users of this
>> feature, so it should be safe to remove it from the PSP uapi.
>> 
>> In theory, the check in the tx-assoc path is more restrictive than
>> necessary. FIN_WAIT1/2, CLOSING, LAST_ACK and CLOSE_WAIT could be
>> allowed, and the peer would accept PSP-encrypted ACKs in the
>> post-FIN-sent states, or data in the half-close case, but it is simpler
>> to disallow those states because they don't fit the upgrade model.
>
> And preferable to do so. While it could be allowed, there is no real
> use case for it. Simpler state model allows simpler code.
>
>> The check in the tx-assoc path fixes a bug in commit 6b46ca260e22 ("net:
>> psp: add socket security association code") where an unsynchronized
>> write can be performed on an assoc shared with a timewait socket when
>> the socket is in TCP_CLOSE after shutdown. This commit is not targeted
>> at net because its premise of preventing listen sockets from holding
>> assoc state depends on net-next commit 8cc3aef0cb19 ("tcp: Do not allow
>> buggy transitions between ehash and lhash2.")
>> 
>> Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
>
> Reviewed-by: Willem de Bruijn <willemb@google.com>
>
> Even if targeting to net-next, you could consider keeping the Fixes tag.
> Importantly the prerequisite patch is mentioned. But is only one of two
> mentioned here?

Yes, sorry. I replied with the full Fixes info to the bot here:
https://lore.kernel.org/netdev/DLTLF0VWT5UY.2WUPQRDZJQO3W@gmail.com/