[PATCH] SUNRPC: Validate TCP record marker length before trusting it

[PATCH] SUNRPC: Validate TCP record marker length before trusting it

[Chu Zhou]

From: Chu Zhou <jack.wemmick@gmail.com>
Subject: [PATCH] SUNRPC: Validate TCP record marker length before trusting it

The SUNRPC/TCP receive parser trusts the 31-bit fragment length of the
record marker (RFC 5531) unconditionally. If the byte stream ever loses
framing sync - for example due to corruption below the RPC layer
(driver / checksum offload / DMA coherence bugs seen on embedded
platforms) or a malfunctioning peer - arbitrary payload bytes are
interpreted as a record marker, and the parser can latch onto a huge
bogus fragment length.

Observed in the field on an OrangePi 5 Plus (Rockchip RK3588) NFSv3/TCP
client: the parser latched onto reclen=0x4DB5ED05 (~1.24 GiB) and
entered the MSG_TRUNC discard path. From that point on, every genuine
reply was discarded as payload of the bogus fragment, with recv.offset
advancing one skb at a time toward a 1.24 GiB boundary that does not
exist in the stream. The TCP connection stayed ESTABLISHED and data
kept flowing, so no socket error or state change ever triggered a
transport reset, and record marking has no resynchronization mechanism.
All RPC tasks piled up in uninterruptible sleep until the hung task
watchdog panicked the machine. Restarting the NFS server (which tears
down the connection) was the only way to recover.

The set of legitimate fragment lengths has a hard upper bound: NFS over
TCP negotiates message sizes of at most 1 MiB (rsize/wsize). Treat any
length above 16 MiB (a generous 16x margin) as proof of framing desync,
emit a tracepoint that captures the parser state for post-mortem
analysis, and fail the read with -ESHUTDOWN so that the connection is
torn down and re-established by the existing transport reset machinery.

The check costs one comparison per fragment.

Fixes: 277e4ab7d530 ("SUNRPC: Simplify TCP receive code by switching to using iterators")
Cc: stable@vger.kernel.org
Tested-by: Chu Zhou <jack.wemmick@gmail.com>
Signed-off-by: Chu Zhou <jack.wemmick@gmail.com>
---
 include/trace/events/sunrpc.h | 30 ++++++++++++++++++++++++++++++
 net/sunrpc/xprtsock.c         | 14 ++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/include/trace/events/sunrpc.h b/include/trace/events/sunrpc.h
index ff85519..d7e2b10 100644
--- a/include/trace/events/sunrpc.h
+++ b/include/trace/events/sunrpc.h
@@ -1370,6 +1370,36 @@ TRACE_EVENT(xs_stream_read_request,
 			__entry->copied, __entry->reclen, __entry->offset)
 );
 
+TRACE_EVENT(xs_stream_bogus_marker,
+	TP_PROTO(const struct sock_xprt *xs),
+
+	TP_ARGS(xs),
+
+	TP_STRUCT__entry(
+		__string(addr, xs->xprt.address_strings[RPC_DISPLAY_ADDR])
+		__string(port, xs->xprt.address_strings[RPC_DISPLAY_PORT])
+		__field(u32, fraghdr)
+		__field(u32, xid)
+		__field(unsigned int, reclen)
+		__field(unsigned int, offset)
+		__field(long, copied)
+	),
+
+	TP_fast_assign(
+		__assign_str(addr);
+		__assign_str(port);
+		__entry->fraghdr = be32_to_cpu(xs->recv.fraghdr);
+		__entry->xid = be32_to_cpu(xs->recv.xid);
+		__entry->reclen = xs->recv.len;
+		__entry->offset = xs->recv.offset;
+		__entry->copied = (long)xs->recv.copied;
+	),
+
+	TP_printk("peer=[%s]:%s fraghdr=0x%08x reclen=%u xid=0x%08x offset=%u copied=%ld",
+		__get_str(addr), __get_str(port), __entry->fraghdr,
+		__entry->reclen, __entry->xid, __entry->offset, __entry->copied)
+);
+
 TRACE_EVENT(rpcb_getport,
 	TP_PROTO(
 		const struct rpc_clnt *clnt,
diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
index 7f60723..cd03dc6 100644
--- a/net/sunrpc/xprtsock.c
+++ b/net/sunrpc/xprtsock.c
@@ -80,6 +80,15 @@ static unsigned int xprt_max_resvport = RPC_DEF_MAX_RESVPORT;
 #define XS_TCP_LINGER_TO	(15U * HZ)
 static unsigned int xs_tcp_fin_timeout __read_mostly = XS_TCP_LINGER_TO;
 
+/*
+ * Largest RPC record marker length we are prepared to trust.
+ * NFS over TCP negotiates message sizes of at most 1 MiB (rsize/wsize),
+ * so this leaves a generous margin. A larger value means the stream
+ * parser has lost framing sync; the only recovery is to reset the
+ * connection, since record marking has no resynchronization mechanism.
+ */
+#define XS_MAX_RECORD_LEN	(16U << 20)
+
 /*
  * We can register our own files under /proc/sys/sunrpc by
  * calling register_sysctl() again.  The files in that
@@ -713,6 +722,11 @@ xs_read_stream(struct sock_xprt *transport, int flags)
 			return transport->recv.offset;
 		transport->recv.len = be32_to_cpu(transport->recv.fraghdr) &
 			RPC_FRAGMENT_SIZE_MASK;
+		if (unlikely(transport->recv.len > XS_MAX_RECORD_LEN)) {
+			trace_xs_stream_bogus_marker(transport);
+			ret = -ESHUTDOWN;
+			goto out_err;
+		}
 		transport->recv.offset -= sizeof(transport->recv.fraghdr);
 		read = ret;
 	}