KCM and kTLS RX do not work together, in either order. KCM's psock_data_ready() is the lower socket's data_ready callback. It can run in softirq context under read_lock_bh(&sk->sk_callback_lock), and calls strp_data_ready(), which invokes sock->ops->read_sock() inline. With kTLS RX, that is tls_sw_read_sock(), which can sleep: it allocates with sk->sk_allocation, waits for async crypto, flushes the socket backlog, and may wait in tls_rx_reader_acquire(). Found by code review. On an unmodified kernel with CONFIG_DEBUG_ATOMIC_SLEEP, sending TLS records over loopback to a socket with software kTLS RX attached to a KCM mux reports: BUG: sleeping function called from invalid context at include/linux/sched/mm.h:321 in_atomic(): 1, irqs_disabled(): 0, non_block: 0, pid: 189, name: ksoftirqd/34 Call Trace: __might_resched kmem_cache_alloc_node_noprof __alloc_skb alloc_skb_with_frags tls_rx_one_record tls_sw_read_sock strp_read_sock strp_data_ready psock_data_ready [kcm] Before commit 662fbcec32f4 ("net/tls: implement ->read_sock()"), TLS inherited tcp_read_sock(), so KCM parsed the ciphertext in the TCP receive queue as messages. HW TLS RX uses the same proto_ops as SW RX, so it is affected the same way. The two also stack socket callbacks and neither restores the other's: with TLS RX configured after the attach, kcm_unattach() overwrites tls_data_ready(), and releasing TLS RX later reinstalls psock_data_ready() on a socket no longer attached to KCM. On TX, kcm_unattach() restores its saved sk_write_space, dropping tls_write_space(). There is no known user of this combination, so reject it, as was done for sockmap in commit 460e6486617c ("tls: reject the combination of TLS and sockmap"). KCM takes over read_sock() and the socket callbacks like a ULP does, so treat it as one: - kcm_attach() refuses sockets that already have any ULP, not only TLS, as sk_psock_init() does for sockmap, with -EOPNOTSUPP like its other refusals; - kcm_attach() then sets icsk_ulp_ops to a dummy, unregistered ULP, so tcp_set_ulp() refuses TLS on an attached socket with -EEXIST and nothing is needed on the TLS side. kcm_unattach() clears it. The dummy is not registered, so TCP_ULP cannot select it by name. Both sides run under the lower socket's lock (tcp_set_ulp() asserts this), so the check is made at ULP install rather than at key configuration. Holding a ULP also has visible side effects: getsockopt(TCP_ULP) and inet_diag report "kcm" on an attached socket, sk_has_decrypt_user() is true so PSP RX association is refused after the attach, and adding an attached socket to a sockmap fails with -EINVAL rather than -EBUSY. This is a user-visible change that will also reach stable kernels: attaching a socket with a ULP now fails with -EOPNOTSUPP, and installing a ULP on an attached socket fails with -EEXIST (espintcp previously failed with -EBUSY). Every refused TLS setup was already broken as described above, except one where only kTLS TX is configured. Such a socket still passes data, because read_sock() stays tcp_read_sock() until kTLS RX is configured, and KCM transmits with sock_sendmsg(), so records reach TLS's sendmsg(). It is still wrong: if TLS was armed before the attach, psock_write_space() shadows tls_write_space() and does not chain to it, so a record deferred on a full send buffer waits for the next send; if TLS was armed after the attach, the chain works until kcm_unattach() drops tls_write_space(), as above. The direction is not known at ULP install, so refusing it keeps one rule for both orders and matches sk_psock_init(). Tested with CONFIG_DEBUG_ATOMIC_SLEEP: both orders are now rejected, plain KCM still delivers messages, and the TLS ULP can be installed after SIOCKCMUNATTACH. Fixes: 662fbcec32f4 ("net/tls: implement ->read_sock()") Cc: stable@vger.kernel.org Suggested-by: Jiayuan Chen Link: https://lore.kernel.org/netdev/3108e934-ec00-458b-8720-450554b35b55@linux.dev/ Signed-off-by: Rishikesh Jethwani --- net/kcm/kcmsock.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c index a7f94d42036f..e0d6536306f4 100644 --- a/net/kcm/kcmsock.c +++ b/net/kcm/kcmsock.c @@ -30,11 +30,19 @@ #include #include #include +#include #include #include unsigned int kcm_net_id; +/* Not registered, so TCP_ULP can't select it. It only holds + * icsk_ulp_ops so tcp_set_ulp() refuses other ULPs like kTLS. + */ +static const struct tcp_ulp_ops kcm_ulp_ops = { + .name = "kcm", +}; + static struct kmem_cache *kcm_psockp __read_mostly; static struct kmem_cache *kcm_muxp __read_mostly; static struct workqueue_struct *kcm_wq; @@ -1272,6 +1280,14 @@ static int kcm_attach(struct socket *sock, struct socket *csock, goto out; } + /* KCM takes over read_sock() and the socket callbacks like a ULP + * does, the two can't stack. + */ + if (inet_csk_has_ulp(csk)) { + err = -EOPNOTSUPP; + goto out; + } + psock = kmem_cache_zalloc(kcm_psockp, GFP_KERNEL); if (!psock) { err = -ENOMEM; @@ -1305,6 +1321,7 @@ static int kcm_attach(struct socket *sock, struct socket *csock, psock->save_write_space = csk->sk_write_space; psock->save_state_change = csk->sk_state_change; csk->sk_user_data = psock; + inet_csk(csk)->icsk_ulp_ops = &kcm_ulp_ops; WRITE_ONCE(csk->sk_data_ready, psock_data_ready); WRITE_ONCE(csk->sk_write_space, psock_write_space); csk->sk_state_change = psock_state_change; @@ -1382,6 +1399,7 @@ static void kcm_unattach(struct kcm_psock *psock) */ write_lock_bh(&csk->sk_callback_lock); csk->sk_user_data = NULL; + inet_csk(csk)->icsk_ulp_ops = NULL; WRITE_ONCE(csk->sk_data_ready, psock->save_data_ready); WRITE_ONCE(csk->sk_write_space, psock->save_write_space); csk->sk_state_change = psock->save_state_change; -- 2.50.1