[PATCH net v2 0/2] net: psp: avoid conflicts with skb->decrypted and sk_validate_xmit_skb()

[PATCH net v2 0/2] net: psp: avoid conflicts with skb->decrypted and sk_validate_xmit_skb()

[Daniel Zahka]

Sashiko's review of commit da630d1da2b1 ("netdevsim: psp: drop tx key
ops") [1] showed that there is a hazard between PSP and offloaded TLS,
where both can clobber what the other set in the sk_validate_xmit_skb
callback.

It was discussed further on the mailing list [2], and it was pointed out
that there are conflicts with PSP and TLS ULP both using the
skb->decrypted bit.

The simplest fix is to make psp and tls mutually exclusive. This series
goes a bit further and makes psp exclusive with all TCP ULPs. The PSP
implementation that we have is not designed to be used with any TCP ULP,
so don't allow a socket to have state for both.

I will send a subsequent series to net-next which will remove the
ability to perform the rx-assoc and tx-assoc psp netlink calls on
sockets that are not in the TCP_ESTABLISHED state. This will close the
remaining quirk that a sk_clone() on a listen socket with psp tx-assoc
state will leave a stale sk->sk_validate_xmit_skb call back on a new,
non-psp socket. I do not believe that change needs to be regarded as a
fix, because it only stands to add unecessary validation code in the tx
path.

[1]: https://sashiko.dev/#/patchset/20260903-psp-prep-v1-0-d47e9c4c375d%40gmail.com
[2]: https://lore.kernel.org/netdev/20260903-psp-prep-v1-0-d47e9c4c375d@gmail.com/

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
Changes in v2:
- remove check on sk->sk_validate_xmit_skb in both tls and psp setup paths 
- Link to v1: https://lore.kernel.org/r/20260910-psp-ktls-fix-v1-0-e3f30aaeca4e@gmail.com

---
Daniel Zahka (2):
      net: psp: avoid conflicts with skb->decrypted and sk_validate_xmit_skb()
      selftests: drv-net: psp: test PSP and TCP ULP mutual exclusion

 include/net/sock.h                         |  2 ++
 net/core/sock.c                            |  7 +++++
 net/ipv4/tcp_ulp.c                         |  4 +++
 net/psp/psp_sock.c                         |  4 +++
 tools/testing/selftests/drivers/net/config |  1 +
 tools/testing/selftests/drivers/net/psp.py | 46 ++++++++++++++++++++++++++++++
 6 files changed, 64 insertions(+)
---
base-commit: 83a945a529d6e002dd7339c532288a931f463dba
change-id: 20260909-psp-ktls-fix-47aa27d955f2

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

[PATCH net v2 1/2] net: psp: avoid conflicts with skb->decrypted and sk_validate_xmit_skb()

[Daniel Zahka]

PSP conflicts with TLS ULP in its usage of both skb->decrypted and
sk->sk_validate_xmit_skb().

Make PSP mutually exclusive with TLS ULP, the only other user of either
of these. As other users of skb->decrypted come along, they can be added
to sk_has_decrypt_user(). It would make sense to also assert that
sk->sk_validate_xmit_skb() is also NULL in both of these setup paths for
similar future proofing, but the PSP listener/sk_clone() path is still
broken and it could be seen as a regression to not allow rx assoc to run
on a child of a listener socket with PSP tx assoc state.

Include all TCP ULPs in the sk_has_decrypt_user() check, even though TLS
is the only one that conflicts with PSP via the decrypted bit. This is
intentional because PSP was not designed to be used with ULPs. It is
best to close off surface area that may make bugs reachable, until
someone wishes to design and test an actual user of PSP with ULPs.

Fixes: 6b46ca260e22 ("net: psp: add socket security association code")
Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
 include/net/sock.h | 2 ++
 net/core/sock.c    | 7 +++++++
 net/ipv4/tcp_ulp.c | 4 ++++
 net/psp/psp_sock.c | 4 ++++
 4 files changed, 17 insertions(+)

diff --git a/include/net/sock.h b/include/net/sock.h
index 51185222aac2..60ea55dc1885 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2312,6 +2312,8 @@ static inline void sk_gso_disable(struct sock *sk)
 	sk->sk_route_caps &= ~NETIF_F_GSO_MASK;
 }
 
+bool sk_has_decrypt_user(const struct sock *sk);
+
 static inline int skb_do_copy_data_nocache(struct sock *sk, struct sk_buff *skb,
 					   struct iov_iter *from, char *to,
 					   int copy, int offset)
diff --git a/net/core/sock.c b/net/core/sock.c
index fa60b7494c58..9489d9c47949 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -142,6 +142,7 @@
 
 #include <trace/events/sock.h>
 
+#include <net/psp.h>
 #include <net/tcp.h>
 #include <net/busy_poll.h>
 #include <net/phonet/phonet.h>
@@ -2670,6 +2671,12 @@ void sk_setup_caps(struct sock *sk, struct dst_entry *dst)
 }
 EXPORT_SYMBOL_GPL(sk_setup_caps);
 
+bool sk_has_decrypt_user(const struct sock *sk)
+{
+	return psp_sk_assoc(sk) ||
+	       (sk_is_inet(sk) && inet_csk_has_ulp(sk)); /* for tls */
+}
+
 /*
  *	Simple resource managers for sockets.
  */
diff --git a/net/ipv4/tcp_ulp.c b/net/ipv4/tcp_ulp.c
index 2aa442128630..b58045df101e 100644
--- a/net/ipv4/tcp_ulp.c
+++ b/net/ipv4/tcp_ulp.c
@@ -136,6 +136,10 @@ static int __tcp_set_ulp(struct sock *sk, const struct tcp_ulp_ops *ulp_ops)
 	if (icsk->icsk_ulp_ops)
 		goto out_err;
 
+	err = -EINVAL;
+	if (sk_has_decrypt_user(sk))
+		goto out_err;
+
 	if (sk->sk_socket)
 		clear_bit(SOCK_SUPPORT_ZC, &sk->sk_socket->flags);
 
diff --git a/net/psp/psp_sock.c b/net/psp/psp_sock.c
index 1a2a6b7516b0..e9b53eedf8db 100644
--- a/net/psp/psp_sock.c
+++ b/net/psp/psp_sock.c
@@ -143,6 +143,10 @@ int psp_sock_assoc_set_rx(struct sock *sk, struct psp_assoc *pas,
 		NL_SET_ERR_MSG(extack, "Socket already has PSP state");
 		err = -EBUSY;
 		goto exit_unlock;
+	} else if (sk_has_decrypt_user(sk)) {
+		NL_SET_ERR_MSG(extack, "Socket has incompatible state");
+		err = -EINVAL;
+		goto exit_unlock;
 	}
 
 	refcount_inc(&pas->refcnt);

-- 
2.52.0

[PATCH net v2 2/2] selftests: drv-net: psp: test PSP and TCP ULP mutual exclusion

[Daniel Zahka]

Test both setting PSP after TLS ULP, and TLS ULP after PSP.

Add CONFIG_TLS=y to the drivers/net/config.

Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>
---
 tools/testing/selftests/drivers/net/config |  1 +
 tools/testing/selftests/drivers/net/psp.py | 46 ++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/tools/testing/selftests/drivers/net/config b/tools/testing/selftests/drivers/net/config
index b6989c7d3d9d..4838adf27fa1 100644
--- a/tools/testing/selftests/drivers/net/config
+++ b/tools/testing/selftests/drivers/net/config
@@ -21,5 +21,6 @@ CONFIG_NET_SCH_INGRESS=y
 CONFIG_NET_SCH_PRIO=m
 CONFIG_PPP=y
 CONFIG_PPPOE=y
+CONFIG_TLS=y
 CONFIG_VLAN_8021Q=m
 CONFIG_XDP_SOCKETS=y
diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
index 315648a770d0..a5b1e14f120f 100755
--- a/tools/testing/selftests/drivers/net/psp.py
+++ b/tools/testing/selftests/drivers/net/psp.py
@@ -23,6 +23,8 @@ from lib.py import NetNSEnter
 from lib.py import bkg, rand_port, wait_port_listen
 from lib.py import ip
 
+TCP_ULP = 31
+
 
 def _get_outq(s):
     one = b'\0' * 4
@@ -333,6 +335,50 @@ def assoc_version_mismatch(cfg):
             ksft_eq(the_exception.nl_msg.error, -errno.EINVAL)
 
 
+def _require_tls_ulp():
+    with socket.create_server(("localhost", 0)) as srv, \
+         socket.create_connection(srv.getsockname()) as s:
+        try:
+            s.setsockopt(socket.SOL_TCP, TCP_ULP, b"tls")
+        except OSError as exc:
+            raise KsftSkipEx("kTLS not available") from exc
+
+
+def assoc_psp_ulp_exclusive(cfg):
+    """ Test that a TCP ULP cannot be attached to a PSP socket """
+    _init_psp_dev(cfg)
+    _require_tls_ulp()
+
+    with _make_clr_conn(cfg) as s:
+        try:
+            cfg.pspnl.rx_assoc({"version": 0,
+                                "dev-id": cfg.psp_dev_id,
+                                "sock-fd": s.fileno()})
+            with ksft_raises(OSError) as cm:
+                s.setsockopt(socket.SOL_TCP, TCP_ULP, b"tls")
+            ksft_eq(cm.exception.errno, errno.EINVAL)
+        finally:
+            _close_conn(cfg, s)
+
+
+def assoc_ulp_psp_exclusive(cfg):
+    """ Test that a PSP assoc cannot be added to a socket with a TCP ULP """
+    _init_psp_dev(cfg)
+    _require_tls_ulp()
+
+    with _make_clr_conn(cfg) as s:
+        try:
+            s.setsockopt(socket.SOL_TCP, TCP_ULP, b"tls")
+            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.EINVAL)
+            ksft_eq(cm.exception.nl_msg.extack['bad-attr'], ".sock-fd")
+        finally:
+            _close_conn(cfg, s)
+
+
 def assoc_twice(cfg):
     """ Test reusing Tx assoc for two sockets """
     _init_psp_dev(cfg)

-- 
2.52.0

Re: [PATCH net v2 1/2] net: psp: avoid conflicts with skb->decrypted and sk_validate_xmit_skb()

[netdev-bot+sashiko]

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

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

- [Low] The patch adds a new, user-visible restriction to two already-
  shipped interfaces without updating Documentation/networking/psp.rst:…

--

net: psp: avoid conflicts with skb->decrypted and sk_validate_xmit_skb()

This makes PSP and TCP ULPs mutually exclusive, since PSP and TLS both use
skb->decrypted and sk->sk_validate_xmit_skb(). A new helper
sk_has_decrypt_user() is added and checked from both __tcp_set_ulp() and
psp_sock_assoc_set_rx().

> diff --git a/net/core/sock.c b/net/core/sock.c
> index fa60b7494c586..9489d9c479493 100644
> --- a/net/core/sock.c
> +++ b/net/core/sock.c

[ ... ]

> @@ -2670,6 +2671,12 @@ void sk_setup_caps(struct sock *sk, struct dst_entry *dst)
>  }
>  EXPORT_SYMBOL_GPL(sk_setup_caps);
>  
> +bool sk_has_decrypt_user(const struct sock *sk)
> +{
> +	return psp_sk_assoc(sk) ||
> +	       (sk_is_inet(sk) && inet_csk_has_ulp(sk)); /* for tls */
> +}
> +
>  /*
>   *	Simple resource managers for sockets.
>   */
> diff --git a/net/ipv4/tcp_ulp.c b/net/ipv4/tcp_ulp.c
> index 2aa442128630e..b58045df101e5 100644
> --- a/net/ipv4/tcp_ulp.c
> +++ b/net/ipv4/tcp_ulp.c
> @@ -136,6 +136,10 @@ static int __tcp_set_ulp(struct sock *sk, const struct tcp_ulp_ops *ulp_ops)
>  	if (icsk->icsk_ulp_ops)
>  		goto out_err;
>  
> +	err = -EINVAL;
> +	if (sk_has_decrypt_user(sk))
> +		goto out_err;
> +
>  	if (sk->sk_socket)
>  		clear_bit(SOCK_SUPPORT_ZC, &sk->sk_socket->flags);
>  
> diff --git a/net/psp/psp_sock.c b/net/psp/psp_sock.c
> index 1a2a6b7516b0f..e9b53eedf8dba 100644
> --- a/net/psp/psp_sock.c
> +++ b/net/psp/psp_sock.c
> @@ -143,6 +143,10 @@ int psp_sock_assoc_set_rx(struct sock *sk, struct psp_assoc *pas,
>  		NL_SET_ERR_MSG(extack, "Socket already has PSP state");
>  		err = -EBUSY;
>  		goto exit_unlock;
> +	} else if (sk_has_decrypt_user(sk)) {
> +		NL_SET_ERR_MSG(extack, "Socket has incompatible state");
> +		err = -EINVAL;
> +		goto exit_unlock;
>  	}

[Severity: Low]
Should Documentation/networking/psp.rst be updated along with this?

Two already-shipped interfaces grow a new failure mode here:
psp_sock_assoc_set_rx() now returns -EINVAL for the rx-assoc netlink
command, and __tcp_set_ulp() now returns -EINVAL for
setsockopt(TCP_ULP).

The "Securing a connection" section enumerates the restrictions of the
rx-assoc/tx-assoc sequence:

	PSP encryption is currently only supported for TCP connections.
	Rx and Tx keys are allocated separately. First the ``rx-assoc``
	Netlink command needs to be issued, specifying a target TCP socket.

but does not mention that a socket with a ULP attached is now rejected.

The "User facing API" section also suggests a TLS-then-PSP bootstrap:

	The expectation is that higher layer protocols will take care of
	protocol and key negotiation. For example one may use TLS key exchange,
	announce the PSP capability, and switch to PSP if both endpoints
	are PSP-capable.

and "Kernel implementation notes" says PSP "follows the TLS offload more
closely ... and the use of skb->decrypted", with nothing stating that kTLS
and PSP can no longer share a socket.

One more thing worth spelling out in the docs: the check is not restricted
to TLS. inet_csk_has_ulp() only tests icsk_ulp_ops, so any registered TCP
ULP blocks rx-assoc, which is broader than the subject line suggests. The
commit message explains this is intentional, but nothing in-tree tells
userspace about it.

The follow-up patch in the series ("selftests: drv-net: psp: test PSP and
TCP ULP mutual exclusion") adds tests for both directions but does not
touch Documentation/, so the gap remains at the end of the series.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915-psp-ktls-fix-v2-0-0eedc3b148ec%40gmail.com

Re: [PATCH net v2 2/2] selftests: drv-net: psp: test PSP and TCP ULP mutual exclusion

[netdev-bot+sashiko]

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

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

- [Medium] In tools/testing/selftests/drivers/net/psp.py, the new helper
  `_require_tls_ulp()` (lines 338-344) wraps…

--

selftests: drv-net: psp: test PSP and TCP ULP mutual exclusion

This adds two psp.py cases that check PSP associations and TCP ULPs are
mutually exclusive in both orders, plus a _require_tls_ulp() helper that
skips when kTLS is unavailable, and enables CONFIG_TLS=y in the
drivers/net selftest config.

> diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
> index 315648a770d00..a5b1e14f120f0 100755
> --- a/tools/testing/selftests/drivers/net/psp.py
> +++ b/tools/testing/selftests/drivers/net/psp.py

[ ... ]

> @@ -333,6 +335,50 @@ def assoc_version_mismatch(cfg):
>              ksft_eq(the_exception.nl_msg.error, -errno.EINVAL)
>  
>  
> +def _require_tls_ulp():
> +    with socket.create_server(("localhost", 0)) as srv, \
> +         socket.create_connection(srv.getsockname()) as s:
> +        try:
> +            s.setsockopt(socket.SOL_TCP, TCP_ULP, b"tls")
> +        except OSError as exc:
> +            raise KsftSkipEx("kTLS not available") from exc

[Severity: Medium]

Should this handler be narrowed to the errnos that actually mean "no
kTLS"?  As written, every OSError from the TCP_ULP setsockopt turns into
a skip, including -EINVAL, which is exactly the errno the kernel side of
this series returns:

net/ipv4/tcp_ulp.c:__tcp_set_ulp() {
	...
	err = -EINVAL;
	if (sk_has_decrypt_user(sk))
		goto out_err;
	...
}

The probe socket here is a plain loopback TCP connection with no PSP
association, so the only legitimate reasons to skip are the "ULP unknown
/ TLS not built" errnos, ENOENT from __tcp_ulp_find_autoload() and
EOPNOTSUPP.

If sk_has_decrypt_user() ever returns true for an ordinary TCP socket -
and the parent commit anticipates growth there ("As other users of
skb->decrypted come along, they can be added to sk_has_decrypt_user()") -
the probe fails with EINVAL and the helper reports a skip.

Would restricting the skip to exc.errno in (errno.ENOENT,
errno.EOPNOTSUPP) and letting other errnos propagate keep the assertions
below load-bearing?

> +
> +
> +def assoc_psp_ulp_exclusive(cfg):
> +    """ Test that a TCP ULP cannot be attached to a PSP socket """
> +    _init_psp_dev(cfg)
> +    _require_tls_ulp()

[Severity: Medium]

Since _require_tls_ulp() is the first gate in both assoc_psp_ulp_exclusive()
and assoc_ulp_psp_exclusive(), does an over-broad kernel-side
sk_has_decrypt_user() end up disabling both directions of the
mutual-exclusion check at once?  The result would be two skips in CI rather
than a failure for the case these tests were added to catch.

> +
> +    with _make_clr_conn(cfg) as s:
> +        try:
> +            cfg.pspnl.rx_assoc({"version": 0,
> +                                "dev-id": cfg.psp_dev_id,
> +                                "sock-fd": s.fileno()})
> +            with ksft_raises(OSError) as cm:
> +                s.setsockopt(socket.SOL_TCP, TCP_ULP, b"tls")
> +            ksft_eq(cm.exception.errno, errno.EINVAL)
> +        finally:
> +            _close_conn(cfg, s)
> +
> +
> +def assoc_ulp_psp_exclusive(cfg):
> +    """ Test that a PSP assoc cannot be added to a socket with a TCP ULP """
> +    _init_psp_dev(cfg)
> +    _require_tls_ulp()

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260915-psp-ktls-fix-v2-0-0eedc3b148ec%40gmail.com

Re: [PATCH net v2 1/2] net: psp: avoid conflicts with skb->decrypted and sk_validate_xmit_skb()

[Daniel Zahka]

On Wed Sep 16, 2026 at 7:13 PM EDT, netdev-bot+sashiko wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
>
> Critical: 0 · High: 0 · Medium: 0 · Low: 1
>
> - [Low] The patch adds a new, user-visible restriction to two already-
>   shipped interfaces without updating Documentation/networking/psp.rst:…
>
> --
>
> net: psp: avoid conflicts with skb->decrypted and sk_validate_xmit_skb()
>
> This makes PSP and TCP ULPs mutually exclusive, since PSP and TLS both use
> skb->decrypted and sk->sk_validate_xmit_skb(). A new helper
> sk_has_decrypt_user() is added and checked from both __tcp_set_ulp() and
> psp_sock_assoc_set_rx().
>
>> diff --git a/net/core/sock.c b/net/core/sock.c
>> index fa60b7494c586..9489d9c479493 100644
>> --- a/net/core/sock.c
>> +++ b/net/core/sock.c
>
> [ ... ]
>
>> @@ -2670,6 +2671,12 @@ void sk_setup_caps(struct sock *sk, struct dst_entry *dst)
>>  }
>>  EXPORT_SYMBOL_GPL(sk_setup_caps);
>>  
>> +bool sk_has_decrypt_user(const struct sock *sk)
>> +{
>> +	return psp_sk_assoc(sk) ||
>> +	       (sk_is_inet(sk) && inet_csk_has_ulp(sk)); /* for tls */
>> +}
>> +
>>  /*
>>   *	Simple resource managers for sockets.
>>   */
>> diff --git a/net/ipv4/tcp_ulp.c b/net/ipv4/tcp_ulp.c
>> index 2aa442128630e..b58045df101e5 100644
>> --- a/net/ipv4/tcp_ulp.c
>> +++ b/net/ipv4/tcp_ulp.c
>> @@ -136,6 +136,10 @@ static int __tcp_set_ulp(struct sock *sk, const struct tcp_ulp_ops *ulp_ops)
>>  	if (icsk->icsk_ulp_ops)
>>  		goto out_err;
>>  
>> +	err = -EINVAL;
>> +	if (sk_has_decrypt_user(sk))
>> +		goto out_err;
>> +
>>  	if (sk->sk_socket)
>>  		clear_bit(SOCK_SUPPORT_ZC, &sk->sk_socket->flags);
>>  
>> diff --git a/net/psp/psp_sock.c b/net/psp/psp_sock.c
>> index 1a2a6b7516b0f..e9b53eedf8dba 100644
>> --- a/net/psp/psp_sock.c
>> +++ b/net/psp/psp_sock.c
>> @@ -143,6 +143,10 @@ int psp_sock_assoc_set_rx(struct sock *sk, struct psp_assoc *pas,
>>  		NL_SET_ERR_MSG(extack, "Socket already has PSP state");
>>  		err = -EBUSY;
>>  		goto exit_unlock;
>> +	} else if (sk_has_decrypt_user(sk)) {
>> +		NL_SET_ERR_MSG(extack, "Socket has incompatible state");
>> +		err = -EINVAL;
>> +		goto exit_unlock;
>>  	}
>
> [Severity: Low]
> Should Documentation/networking/psp.rst be updated along with this?
>
> Two already-shipped interfaces grow a new failure mode here:
> psp_sock_assoc_set_rx() now returns -EINVAL for the rx-assoc netlink
> command, and __tcp_set_ulp() now returns -EINVAL for
> setsockopt(TCP_ULP).
>
> The "Securing a connection" section enumerates the restrictions of the
> rx-assoc/tx-assoc sequence:
>
> 	PSP encryption is currently only supported for TCP connections.
> 	Rx and Tx keys are allocated separately. First the ``rx-assoc``
> 	Netlink command needs to be issued, specifying a target TCP socket.
>
> but does not mention that a socket with a ULP attached is now rejected.
>
> The "User facing API" section also suggests a TLS-then-PSP bootstrap:
>
> 	The expectation is that higher layer protocols will take care of
> 	protocol and key negotiation. For example one may use TLS key exchange,
> 	announce the PSP capability, and switch to PSP if both endpoints
> 	are PSP-capable.
>
> and "Kernel implementation notes" says PSP "follows the TLS offload more
> closely ... and the use of skb->decrypted", with nothing stating that kTLS
> and PSP can no longer share a socket.
>
> One more thing worth spelling out in the docs: the check is not restricted
> to TLS. inet_csk_has_ulp() only tests icsk_ulp_ops, so any registered TCP
> ULP blocks rx-assoc, which is broader than the subject line suggests. The
> commit message explains this is intentional, but nothing in-tree tells
> userspace about it.
>
> The follow-up patch in the series ("selftests: drv-net: psp: test PSP and
> TCP ULP mutual exclusion") adds tests for both directions but does not
> touch Documentation/, so the gap remains at the end of the series.

I can incorporate some changes to psp.rst into a v3 if others think
that improves the quality of this patch and the PSP code. Personally, I
don't think this suggestion is worth while. What this code prevents is
something I would be surprised if a user were to try without heavily
inspecting both the implementations of PSP and TLS ULP first, at which
point I don't know if using up words in this doc is worth it.

Re: [PATCH net v2 2/2] selftests: drv-net: psp: test PSP and TCP ULP mutual exclusion

[Daniel Zahka]

On Wed Sep 16, 2026 at 7:13 PM EDT, netdev-bot+sashiko wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
>
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
>
> - [Medium] In tools/testing/selftests/drivers/net/psp.py, the new helper
>   `_require_tls_ulp()` (lines 338-344) wraps…
>
> --
>
> selftests: drv-net: psp: test PSP and TCP ULP mutual exclusion
>
> This adds two psp.py cases that check PSP associations and TCP ULPs are
> mutually exclusive in both orders, plus a _require_tls_ulp() helper that
> skips when kTLS is unavailable, and enables CONFIG_TLS=y in the
> drivers/net selftest config.
>
>> diff --git a/tools/testing/selftests/drivers/net/psp.py b/tools/testing/selftests/drivers/net/psp.py
>> index 315648a770d00..a5b1e14f120f0 100755
>> --- a/tools/testing/selftests/drivers/net/psp.py
>> +++ b/tools/testing/selftests/drivers/net/psp.py
>
> [ ... ]
>
>> @@ -333,6 +335,50 @@ def assoc_version_mismatch(cfg):
>>              ksft_eq(the_exception.nl_msg.error, -errno.EINVAL)
>>  
>>  
>> +def _require_tls_ulp():
>> +    with socket.create_server(("localhost", 0)) as srv, \
>> +         socket.create_connection(srv.getsockname()) as s:
>> +        try:
>> +            s.setsockopt(socket.SOL_TCP, TCP_ULP, b"tls")
>> +        except OSError as exc:
>> +            raise KsftSkipEx("kTLS not available") from exc
>
> [Severity: Medium]
>

I understand the point, but I'm not sure this really worth the change. I
considered checking for specific error codes here, but decided against
it because maybe that would be end up being brittle if someone in the
future changed or added error codes. Aside from ENOENT when the feature
is not available, I think the only other error this could throw is
ENOMEM.

> Should this handler be narrowed to the errnos that actually mean "no
> kTLS"?  As written, every OSError from the TCP_ULP setsockopt turns into
> a skip, including -EINVAL, which is exactly the errno the kernel side of
> this series returns:
>
> net/ipv4/tcp_ulp.c:__tcp_set_ulp() {
> 	...
> 	err = -EINVAL;
> 	if (sk_has_decrypt_user(sk))
> 		goto out_err;
> 	...
> }
>
> The probe socket here is a plain loopback TCP connection with no PSP
> association, so the only legitimate reasons to skip are the "ULP unknown
> / TLS not built" errnos, ENOENT from __tcp_ulp_find_autoload() and
> EOPNOTSUPP.
>
> If sk_has_decrypt_user() ever returns true for an ordinary TCP socket -

The code in question can't take this branch as it is now, and if it did,
a skip seems arguably more appropriate to me, because otherwise how
would we even interpret the result of the test.

> and the parent commit anticipates growth there ("As other users of
> skb->decrypted come along, they can be added to sk_has_decrypt_user()") -
> the probe fails with EINVAL and the helper reports a skip.
>
> Would restricting the skip to exc.errno in (errno.ENOENT,
> errno.EOPNOTSUPP) and letting other errnos propagate keep the assertions
> below load-bearing?
>
>> +
>> +
>> +def assoc_psp_ulp_exclusive(cfg):
>> +    """ Test that a TCP ULP cannot be attached to a PSP socket """
>> +    _init_psp_dev(cfg)
>> +    _require_tls_ulp()
>
> [Severity: Medium]
>
> Since _require_tls_ulp() is the first gate in both assoc_psp_ulp_exclusive()
> and assoc_ulp_psp_exclusive(), does an over-broad kernel-side
> sk_has_decrypt_user() end up disabling both directions of the
> mutual-exclusion check at once?  The result would be two skips in CI rather
> than a failure for the case these tests were added to catch.
>
>> +
>> +    with _make_clr_conn(cfg) as s:
>> +        try:
>> +            cfg.pspnl.rx_assoc({"version": 0,
>> +                                "dev-id": cfg.psp_dev_id,
>> +                                "sock-fd": s.fileno()})
>> +            with ksft_raises(OSError) as cm:
>> +                s.setsockopt(socket.SOL_TCP, TCP_ULP, b"tls")
>> +            ksft_eq(cm.exception.errno, errno.EINVAL)
>> +        finally:
>> +            _close_conn(cfg, s)
>> +
>> +
>> +def assoc_ulp_psp_exclusive(cfg):
>> +    """ Test that a PSP assoc cannot be added to a socket with a TCP ULP """
>> +    _init_psp_dev(cfg)
>> +    _require_tls_ulp()
>
> [ ... ]

Re: [PATCH net v2 1/2] net: psp: avoid conflicts with skb->decrypted and sk_validate_xmit_skb()

[Willem de Bruijn]

On Wed, Sep 16, 2026 at 7:39 PM Daniel Zahka <daniel.zahka@gmail.com> wrote:
>
> On Wed Sep 16, 2026 at 7:13 PM EDT, netdev-bot+sashiko wrote:
> > Thank you for your contribution! Sashiko AI review found 1 potential
> > issue(s) to consider:
> >
> > Critical: 0 · High: 0 · Medium: 0 · Low: 1
> >
> > - [Low] The patch adds a new, user-visible restriction to two already-
> >   shipped interfaces without updating Documentation/networking/psp.rst:…
> >
> > --
> >
> > net: psp: avoid conflicts with skb->decrypted and sk_validate_xmit_skb()
> >
> > This makes PSP and TCP ULPs mutually exclusive, since PSP and TLS both use
> > skb->decrypted and sk->sk_validate_xmit_skb(). A new helper
> > sk_has_decrypt_user() is added and checked from both __tcp_set_ulp() and
> > psp_sock_assoc_set_rx().
> >
> >> diff --git a/net/core/sock.c b/net/core/sock.c
> >> index fa60b7494c586..9489d9c479493 100644
> >> --- a/net/core/sock.c
> >> +++ b/net/core/sock.c
> >
> > [ ... ]
> >
> >> @@ -2670,6 +2671,12 @@ void sk_setup_caps(struct sock *sk, struct dst_entry *dst)
> >>  }
> >>  EXPORT_SYMBOL_GPL(sk_setup_caps);
> >>
> >> +bool sk_has_decrypt_user(const struct sock *sk)
> >> +{
> >> +    return psp_sk_assoc(sk) ||
> >> +           (sk_is_inet(sk) && inet_csk_has_ulp(sk)); /* for tls */
> >> +}
> >> +
> >>  /*
> >>   *  Simple resource managers for sockets.
> >>   */
> >> diff --git a/net/ipv4/tcp_ulp.c b/net/ipv4/tcp_ulp.c
> >> index 2aa442128630e..b58045df101e5 100644
> >> --- a/net/ipv4/tcp_ulp.c
> >> +++ b/net/ipv4/tcp_ulp.c
> >> @@ -136,6 +136,10 @@ static int __tcp_set_ulp(struct sock *sk, const struct tcp_ulp_ops *ulp_ops)
> >>      if (icsk->icsk_ulp_ops)
> >>              goto out_err;
> >>
> >> +    err = -EINVAL;
> >> +    if (sk_has_decrypt_user(sk))
> >> +            goto out_err;
> >> +
> >>      if (sk->sk_socket)
> >>              clear_bit(SOCK_SUPPORT_ZC, &sk->sk_socket->flags);
> >>
> >> diff --git a/net/psp/psp_sock.c b/net/psp/psp_sock.c
> >> index 1a2a6b7516b0f..e9b53eedf8dba 100644
> >> --- a/net/psp/psp_sock.c
> >> +++ b/net/psp/psp_sock.c
> >> @@ -143,6 +143,10 @@ int psp_sock_assoc_set_rx(struct sock *sk, struct psp_assoc *pas,
> >>              NL_SET_ERR_MSG(extack, "Socket already has PSP state");
> >>              err = -EBUSY;
> >>              goto exit_unlock;
> >> +    } else if (sk_has_decrypt_user(sk)) {
> >> +            NL_SET_ERR_MSG(extack, "Socket has incompatible state");
> >> +            err = -EINVAL;
> >> +            goto exit_unlock;
> >>      }
> >
> > [Severity: Low]
> > Should Documentation/networking/psp.rst be updated along with this?
> >
> > Two already-shipped interfaces grow a new failure mode here:
> > psp_sock_assoc_set_rx() now returns -EINVAL for the rx-assoc netlink
> > command, and __tcp_set_ulp() now returns -EINVAL for
> > setsockopt(TCP_ULP).
> >
> > The "Securing a connection" section enumerates the restrictions of the
> > rx-assoc/tx-assoc sequence:
> >
> >       PSP encryption is currently only supported for TCP connections.
> >       Rx and Tx keys are allocated separately. First the ``rx-assoc``
> >       Netlink command needs to be issued, specifying a target TCP socket.
> >
> > but does not mention that a socket with a ULP attached is now rejected.
> >
> > The "User facing API" section also suggests a TLS-then-PSP bootstrap:
> >
> >       The expectation is that higher layer protocols will take care of
> >       protocol and key negotiation. For example one may use TLS key exchange,
> >       announce the PSP capability, and switch to PSP if both endpoints
> >       are PSP-capable.
> >
> > and "Kernel implementation notes" says PSP "follows the TLS offload more
> > closely ... and the use of skb->decrypted", with nothing stating that kTLS
> > and PSP can no longer share a socket.
> >
> > One more thing worth spelling out in the docs: the check is not restricted
> > to TLS. inet_csk_has_ulp() only tests icsk_ulp_ops, so any registered TCP
> > ULP blocks rx-assoc, which is broader than the subject line suggests. The
> > commit message explains this is intentional, but nothing in-tree tells
> > userspace about it.
> >
> > The follow-up patch in the series ("selftests: drv-net: psp: test PSP and
> > TCP ULP mutual exclusion") adds tests for both directions but does not
> > touch Documentation/, so the gap remains at the end of the series.
>
> I can incorporate some changes to psp.rst into a v3 if others think
> that improves the quality of this patch and the PSP code. Personally, I
> don't think this suggestion is worth while. What this code prevents is
> something I would be surprised if a user were to try without heavily
> inspecting both the implementations of PSP and TLS ULP first, at which
> point I don't know if using up words in this doc is worth it.

Doesn't seem worthwhile to me either

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

Re: [PATCH net v2 2/2] selftests: drv-net: psp: test PSP and TCP ULP mutual exclusion

[Willem de Bruijn]

On Tue, Sep 15, 2026 at 7:11 PM Daniel Zahka <daniel.zahka@gmail.com> wrote:
>
> Test both setting PSP after TLS ULP, and TLS ULP after PSP.
>
> Add CONFIG_TLS=y to the drivers/net/config.
>
> Signed-off-by: Daniel Zahka <daniel.zahka@gmail.com>

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

Re: [PATCH net v2 0/2] net: psp: avoid conflicts with skb->decrypted and sk_validate_xmit_skb()

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 15 Sep 2026 16:11:36 -0700 you wrote:
> Sashiko's review of commit da630d1da2b1 ("netdevsim: psp: drop tx key
> ops") [1] showed that there is a hazard between PSP and offloaded TLS,
> where both can clobber what the other set in the sk_validate_xmit_skb
> callback.
> 
> It was discussed further on the mailing list [2], and it was pointed out
> that there are conflicts with PSP and TLS ULP both using the
> skb->decrypted bit.
> 
> [...]

Here is the summary with links:
  - [net,v2,1/2] net: psp: avoid conflicts with skb->decrypted and sk_validate_xmit_skb()
    https://git.kernel.org/netdev/net/c/a41f24c612c3
  - [net,v2,2/2] selftests: drv-net: psp: test PSP and TCP ULP mutual exclusion
    https://git.kernel.org/netdev/net/c/b4288c59bda8

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html