[PATCH] rpmsg: virtio: Reject a message shorter than its header

[Lianjie Wang]

rpmsg_recv_single() rejects a message whose msg_len exceeds len -
sizeof(struct rpmsg_hdr), where len is the used length the device
reported. Since len is an unsigned int, a used length shorter than the
header wraps the difference, and every msg_len passes. The endpoint
callback receives up to 65535 bytes from a receive buffer of 512 bytes
by default, and rpmsg_char copies them into an skb that userspace reads.
The receive and transmit buffers share one DMA allocation, so the copy
can run through the following buffers and, with small vrings, past the
end of the allocation. With vrings of 16 buffers and a header claiming
65535 bytes, KASAN reports:

  BUG: KASAN: slab-out-of-bounds in rpmsg_ept_cb+0x4d/0xb0
  Read of size 65535 at addr ffff888002484010 by task rpmsg_echo/140
  Call Trace:
   rpmsg_ept_cb+0x4d/0xb0
   rpmsg_recv_done+0x29f/0x840
   vring_interrupt+0x32a/0x540
   vp_interrupt+0x146/0x180

Compare the sum of the header size and msg_len with len. The sum cannot
overflow, since msg_len has 16 bits. Move the check ahead of the debug
output, since dynamic_hex_dump() reads sizeof(*msg) + msg_len bytes
when its dynamic debug site is enabled.

This patch is assisted by Claude Opus 5.5.

Fixes: 9648224e564a ("rpmsg: validate incoming message length before propagating")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Lianjie Wang <lian@xxxxxxxxxxxxxxxxxx>
---
 drivers/rpmsg/virtio_rpmsg_bus.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 23ff6c8b7a20b..a9125e81cb150 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -661,6 +661,16 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
 	unsigned int msg_len = __rpmsg16_to_cpu(little_endian, msg->len);
 	int err;
 
+	/*
+	 * We currently use fixed-sized buffers, so trivially sanitize
+	 * the reported payload length.
+	 */
+	if (len > vrp->rx_buf_size ||
+	    sizeof(struct rpmsg_hdr) + msg_len > len) {
+		dev_warn(dev, "inbound msg too big: (%d, %d)\n", len, msg_len);
+		return -EINVAL;
+	}
+
 	dev_dbg(dev, "From: 0x%x, To: 0x%x, Len: %d, Flags: %d, Reserved: %d\n",
 		__rpmsg32_to_cpu(little_endian, msg->src),
 		__rpmsg32_to_cpu(little_endian, msg->dst), msg_len,
@@ -671,16 +681,6 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
 			 msg, sizeof(*msg) + msg_len, true);
 #endif
 
-	/*
-	 * We currently use fixed-sized buffers, so trivially sanitize
-	 * the reported payload length.
-	 */
-	if (len > vrp->rx_buf_size ||
-	    msg_len > (len - sizeof(struct rpmsg_hdr))) {
-		dev_warn(dev, "inbound msg too big: (%d, %d)\n", len, msg_len);
-		return -EINVAL;
-	}
-
 	/* use the dst addr to fetch the callback of the appropriate user */
 	mutex_lock(&vrp->endpoints_lock);
 

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4
-- 
2.56.0