On Wed, Sep 16, 2026 at 7:39 PM Daniel Zahka <[email protected]> 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 <[email protected]>
