[PATCH net v2 1/2] kcm: defer receive parsing when the lower socket's read_sock may sleep

[Rishikesh Jethwani <rjethwani@purestorage.com>]

psock_data_ready() is the lower socket's data_ready callback and can
run in softirq context, under read_lock_bh(&sk->sk_callback_lock). It calls
strp_data_ready(), which, when the socket is not owned by a user,
invokes sock->ops->read_sock() inline to pull records out of the
receive queue. For a plain TCP socket that is tcp_read_sock(), which
does not block.

A ULP may install a different read_sock. kTLS installs
tls_sw_read_sock(), which can sleep: it waits for an asynchronous
AEAD to complete via crypto_wait_req(), flushes the socket backlog
through __sk_flush_backlog(), allocates with sk->sk_allocation, and,
on reader contention in tls_rx_reader_acquire(), enters
sk_wait_event()'s socket release/reacquire path despite the zero
timeout. It passes nonblock, so it does not wait for data, but these
paths can require sleepable context, which softirq and a BH read lock
do not provide. Attaching a kTLS socket to a KCM multiplexor can
therefore sleep in atomic context.

With CONFIG_DEBUG_ATOMIC_SLEEP, a loopback TCP connection using software
kTLS RX, with the receiving socket 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]

Keep the inline fast path for tcp_read_sock() and hand any other
read_sock to the parser work via strp_check_rcv(). The work runs
do_strp_work() under lock_sock(), where a sleeping read_sock is fine.
sk->sk_socket should not be NULL here, since KCM holds the lower
socket's file reference while a psock is attached; if it is,
defer as well, as strp_read_sock() would not read without a socket.

The deferred path also skips strp_data_ready()'s need_bytes early
return. That check counts bytes in the TCP receive queue, which for
kTLS is ciphertext rather than parsed records, so it would not be
meaningful there. Without it, a data_ready event can queue work that
finds no complete record and returns; queue_work() does nothing if the
work is already pending.

kTLS installs tls_sw_read_sock() by replacing sk_socket->ops with
WRITE_ONCE() in update_sk_prot() when TLS_RX is configured. That happens
under lock_sock() only, and can happen after the socket is attached to
KCM, so read it with READ_ONCE().

The deferred work runs on kstrp, which is currently an ordered workqueue
shared by all strparser users. The next patch makes it per-CPU, so a
psock whose read_sock does software decryption does not hold up parsing
for other strparser users.

Tested with the reproducer above: 5/5 runs produce no splat and full
datagram delivery; without this change every run hits "BUG: sleeping
function called from invalid context" in strp_data_ready ->
tls_sw_read_sock.

Multi-record messages (>16 KiB each) also deliver completely when kTLS
RX is installed before the KCM attach and the entire stream is queued
before the reader starts: the parser correctly pauses on KCM flow
control (psock RxPause/RdyRx) and resumes on read, with no strparser
need_bytes stall.

Fixes: 662fbcec32f4 ("net/tls: implement ->read_sock()")
Signed-off-by: Rishikesh Jethwani <rjethwani@purestorage.com>
---
 net/kcm/kcmsock.c | 33 +++++++++++++++++++++++++++++++--
 1 file changed, 31 insertions(+), 2 deletions(-)

diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
index 71af69d442f2..ec7aaba17fe6 100644
--- a/net/kcm/kcmsock.c
+++ b/net/kcm/kcmsock.c
@@ -30,6 +30,7 @@
 #include <net/kcm.h>
 #include <net/netns/generic.h>
 #include <net/sock.h>
+#include <net/tcp.h>
 #include <uapi/linux/kcm.h>
 #include <trace/events/sock.h>
 
@@ -348,6 +349,30 @@ static void unreserve_rx_kcm(struct kcm_psock *psock,
 	spin_unlock_bh(&mux->rx_lock);
 }
 
+/* strp_data_ready() calls the lower socket's read_sock() inline, from
+ * softirq and under sk_callback_lock. That is only safe for TCP's own
+ * tcp_read_sock(); anything else goes to the parser work, which runs
+ * under lock_sock(). Called with sk_callback_lock held.
+ */
+static bool psock_can_parse_inline(struct sock *sk)
+{
+	struct socket *sock = sk->sk_socket;
+	const struct proto_ops *ops;
+
+	/* KCM holds the lower socket's file reference while a psock is
+	 * attached, so sk_socket should not be NULL here. Defer if it is,
+	 * as strp_read_sock() would not read without a socket anyway.
+	 */
+	if (!sock)
+		return false;
+
+	/* A ULP's read_sock, such as kTLS's tls_sw_read_sock(), may sleep.
+	 * READ_ONCE() pairs with the WRITE_ONCE() in kTLS's update_sk_prot().
+	 */
+	ops = READ_ONCE(sock->ops);
+	return ops->read_sock == tcp_read_sock;
+}
+
 /* Lower sock lock held */
 static void psock_data_ready(struct sock *sk)
 {
@@ -358,8 +383,12 @@ static void psock_data_ready(struct sock *sk)
 	read_lock_bh(&sk->sk_callback_lock);
 
 	psock = (struct kcm_psock *)sk->sk_user_data;
-	if (likely(psock))
-		strp_data_ready(&psock->strp);
+	if (likely(psock)) {
+		if (psock_can_parse_inline(sk))
+			strp_data_ready(&psock->strp);
+		else
+			strp_check_rcv(&psock->strp);
+	}
 
 	read_unlock_bh(&sk->sk_callback_lock);
 }
-- 
2.50.1