[PATCH net 0/2] tls: fix plaintext sk_msg ring over-fill in tls_sw_sendmsg_splice()

[PATCH net 0/2] tls: fix plaintext sk_msg ring over-fill in tls_sw_sendmsg_splice()

[chanyoung]

An unprivileged local user can oops the kernel by splicing into a kTLS
socket whose open record already has a full plaintext scatterlist ring.

sk_msg_page_add() has no fullness check of its own and
tls_sw_sendmsg_splice() only tests sk_msg_full() at the bottom of its
do-while loop, so a page is always added before the ring is checked.  That
first add writes the reserved slot and wraps sg.end onto sg.start;
sk_msg_iter_dist() then returns 0, sk_msg_full() reports the full ring as
empty, and the loop keeps overwriting live entries while sg.size grows.
Pushing the resulting record drives the AEAD with a cryptlen larger than
the walkable scatterlist, so the walk runs past the end-marked entry and
dereferences the NULL returned by sg_next().

The ring is left full and unpushed across a syscall by the copy path,
which does not set full_record when the fragment it adds is the one that
exactly fills the ring.

No capabilities, namespaces, io_uring, nftables or BPF are involved; a
plain loopback TCP socket with the "tls" ULP attached is enough.
Reproduced as an ordinary user (uid 1000) on stock production configs --
CONFIG_TLS=y, no KASAN, no LOCKDEP, no fault injection -- on 7.2.0-rc4 and
on 6.12.96.  Present since v6.5.

Patch 1 evaluates the loop condition before the first sk_msg_page_add()
instead of after it.  Patch 2 adds a regression test: it oopses an
unpatched kernel and passes with patch 1 applied (924/924 for the whole
tls suite, with CONFIG_CRYPTO_CHACHA20POLY1305, CONFIG_CRYPTO_SM4 and
CONFIG_CRYPTO_ARIA enabled so every cipher variant runs).

I found this with AI assistance, so per
Documentation/process/security-bugs.rst I am treating it as public and
posting here rather than sending it to security@kernel.org.  For the same
reason I am not posting the standalone reproducer; I can send it privately
if that would be useful.

chanyoung (2):
  tls: don't over-fill the plaintext sk_msg ring in
    tls_sw_sendmsg_splice()
  selftests: tls: add a test for splicing onto a full plaintext record

 net/tls/tls_sw.c                  |  4 +--
 tools/testing/selftests/net/tls.c | 52 +++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+), 2 deletions(-)

-- 
2.43.0

[PATCH net 1/2] tls: don't over-fill the plaintext sk_msg ring in tls_sw_sendmsg_splice()

[chanyoung]

tls_sw_sendmsg_splice() appends pages to the open record's plaintext
sk_msg ring with sk_msg_page_add(), which performs no fullness check of
its own, and the loop only tests sk_msg_full() at the bottom of its
do-while.

If the ring is already full when the function is entered, the first
sk_msg_page_add() writes the reserved slot and sk_msg_iter_next() wraps
sg.end around to sg.start.  sk_msg_iter_dist() then returns 0, so
sk_msg_full() reports the ring as empty, the loop keeps running, and each
further add overwrites a live entry without putting its page reference
while sg.size keeps growing.  sg.size is then larger than the data
reachable by walking the logical [sg.start, sg.end) ring.

tls_push_record() marks the end of the scatterlist at the logical last
entry but passes the inflated msg_pl->sg.size to tls_do_encryption() as
cryptlen, so the AEAD scatterwalk runs past the end-marked entry and
dereferences the NULL returned by sg_next():

  BUG: kernel NULL pointer dereference, address: 0000000000000008
  CPU: 1 UID: 1000 PID: 204 Comm: exploit Not tainted 7.2.0-rc4+ #1 PREEMPTLAZY
  RIP: 0010:memcpy_from_scatterwalk+0x32/0xc0
  Call Trace:
   <TASK>
   skcipher_walk_next+0x1d1/0x2c0
   gcm_encrypt_aesni_avx+0x1e9/0x220
   bpf_exec_tx_verdict+0x3bb/0x860
   tls_sw_sendmsg+0xa1a/0xca0
   __sys_sendto+0x1da/0x1f0
   do_syscall_64+0xdc/0x520
   entry_SYSCALL_64_after_hwframe+0x76/0x7e
   </TASK>

An unprivileged user can reach this on a plain loopback TCP socket with
the "tls" ULP attached.  A full but unpushed plaintext ring survives
across a syscall through the copy path: sk_msg_clone() returns 0 rather
than -ENOSPC for the frag that makes the ring exactly full, because its
guard is "if (i == src->sg.end && len)" and len reaches 0 as that frag is
added, so full_record is never set and MSG_MORE keeps eor clear.  Since
record_room is a byte count, a frag-exhausted ring that holds only a few
hundred bytes still admits the next splice(), which then re-enters
tls_sw_sendmsg_splice() on a full ring.

The caller already handles a ring that becomes full during the splice by
testing sk_msg_full() afterwards and setting full_record to push the
record, so the loop condition only needs to be evaluated before the first
sk_msg_page_add() rather than after it.  Turn the do-while into a while
loop: when the ring is full on entry the function returns without adding
anything, the caller pushes the record, and the next iteration of the
caller's loop starts from a fresh, empty ring.

Cc: stable@vger.kernel.org
Fixes: fe1e81d4f73b ("tls/sw: Support MSG_SPLICE_PAGES")
Signed-off-by: Chanyoung Park <ppoo1220@gmail.com>
---
 net/tls/tls_sw.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index d4afc90fd79..0c413d05bb1 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -738,7 +738,7 @@ static int tls_sw_sendmsg_splice(struct sock *sk, struct msghdr *msg,
 {
 	struct page *page = NULL, **pages = &page;
 
-	do {
+	while (try_to_copy && !sk_msg_full(msg_pl)) {
 		ssize_t part;
 		size_t off;
 
@@ -758,7 +758,7 @@ static int tls_sw_sendmsg_splice(struct sock *sk, struct msghdr *msg,
 		sk_mem_charge(sk, part);
 		*copied += part;
 		try_to_copy -= part;
-	} while (try_to_copy && !sk_msg_full(msg_pl));
+	}
 
 	return 0;
 }
-- 
2.43.0

[PATCH net 2/2] selftests: tls: add a test for splicing onto a full plaintext record

[chanyoung]

Splicing into an open record whose plaintext scatterlist ring was already
full used to wrap the ring's end index onto its start.  sk_msg_full() then
reported the full ring as empty, so the loop kept overwriting live entries
while sg.size grew, and pushing the record ran the AEAD scatterwalk off the
end of the scatterlist.

Reaching that state needs the ring to be left full and unpushed across a
syscall, which the copy path does when the fragment it adds is the one that
fills the ring.  The test therefore fills the ring with splices, adds one
byte with MSG_MORE, splices some more, and only then pushes the record.

MAX_SKB_FRAGS is configurable, so rather than hardcoding the number of
fragments needed to fill the ring, sweep it over the plausible range so the
one-byte send lands exactly on a full ring whatever the kernel was built
with.

On an unpatched kernel this oopses in the AEAD walk; with the preceding
patch applied the whole tls selftest suite passes.

Signed-off-by: Chanyoung Park <ppoo1220@gmail.com>
---
 tools/testing/selftests/net/tls.c | 52 +++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/tools/testing/selftests/net/tls.c b/tools/testing/selftests/net/tls.c
index cbdd3ea28b9..d2666884ea8 100644
--- a/tools/testing/selftests/net/tls.c
+++ b/tools/testing/selftests/net/tls.c
@@ -835,6 +835,58 @@ TEST_F(tls, send_and_splice)
 	EXPECT_EQ(memcmp(mem_send, mem_recv, send_len), 0);
 }
 
+/* Splicing into an open record whose plaintext scatterlist ring is already
+ * full used to wrap the ring's end index onto its start, after which
+ * sk_msg_full() reported the full ring as empty: further pages overwrote
+ * live entries and sg.size desynced from the walkable scatterlist, which
+ * oopsed in the AEAD walk once the record was pushed.  The ring is left
+ * full and unpushed by the copy path, which does not push the record when
+ * the fragment it adds is the one that fills the ring.
+ */
+TEST_F(tls, splice_onto_full_record)
+{
+	int frag_len = 100, extra = 4;
+	char mem_send[5000];
+	char mem_recv[5000];
+	int nfrags, i, total;
+	int p[2];
+
+	memrnd(mem_send, sizeof(mem_send));
+
+	/* MAX_SKB_FRAGS is configurable (17 by default), so sweep the
+	 * plausible range to land the one-byte send exactly on a full ring
+	 * whatever this kernel was built with.
+	 */
+	for (nfrags = 12; nfrags <= 45; nfrags++) {
+		total = (nfrags + extra) * frag_len + 2;
+
+		for (i = 0; i < nfrags; i++) {
+			ASSERT_GE(pipe(p), 0);
+			EXPECT_EQ(write(p[1], mem_send, frag_len), frag_len);
+			EXPECT_EQ(splice(p[0], NULL, self->fd, NULL, frag_len,
+					 SPLICE_F_MORE), frag_len);
+			close(p[0]);
+			close(p[1]);
+		}
+
+		EXPECT_EQ(send(self->fd, mem_send, 1, MSG_MORE), 1);
+
+		for (i = 0; i < extra; i++) {
+			ASSERT_GE(pipe(p), 0);
+			EXPECT_EQ(write(p[1], mem_send, frag_len), frag_len);
+			EXPECT_EQ(splice(p[0], NULL, self->fd, NULL, frag_len,
+					 SPLICE_F_MORE), frag_len);
+			close(p[0]);
+			close(p[1]);
+		}
+
+		EXPECT_EQ(send(self->fd, mem_send, 1, 0), 1);
+
+		EXPECT_EQ(recv(self->cfd, mem_recv, total, MSG_WAITALL), total);
+		EXPECT_EQ(memcmp(mem_send, mem_recv, frag_len), 0);
+	}
+}
+
 TEST_F(tls, splice_to_pipe)
 {
 	int send_len = TLS_PAYLOAD_MAX_LEN;
-- 
2.43.0