Skip to contentSkip to conversation

rpmsg: virtio: Reject a message shorter than its header

PATCH Applied Lianjie Wang 2 participants 2 messages Nov 6 – Nov 9

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 rpmsg: virtio: Reject a message shorter than its header

rpmsg_recv_single() checks the msg_len field of a message header against the used length with an unsigned subtraction, so every msg_len passes the check with a used length shorter than the header. The rx callback of the destination endpoint can receive a message of up to 65535 bytes, longer than the default receive buffer of 512 bytes, so rpmsg_char can copy the whole message into an skb and read past the buffer:

  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 the used length, so no subtraction wraps.

This patch is assisted by Claude Opus 5.5.

Fixes 9648224e564a ("rpmsg: validate incoming message length before propagating")

---
v2:
- Leave the check after the dynamic_hex_dump statement.
v1: https://lore.kernel.org/all/[email protected]/

 drivers/rpmsg/virtio_rpmsg_bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/rpmsg/virtio_rpmsg_bus.c +1 -1
index 23ff6c8b7a20b..3d58bb19e9297 100644
@@ -676,7 +676,7 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
 	 * the reported payload length.
 	 */
 	if (len > vrp->rx_buf_size ||
-	    msg_len > (len - sizeof(struct rpmsg_hdr))) {
+	    sizeof(struct rpmsg_hdr) + msg_len > len) {
 		dev_warn(dev, "inbound msg too big: (%d, %d)\n", len, msg_len);
 		return -EINVAL;
 	}

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4

Signature · 1 line
2.56.0
maintainer Applied
#1 lore raw
Quoted 36 lines · Lianjie Wang

rpmsg_recv_single() checks the msg_len field of a message header against the used length with an unsigned subtraction, so every msg_len passes the check with a used length shorter than the header. The rx callback of the destination endpoint can receive a message of up to 65535 bytes, longer than the default receive buffer of 512 bytes, so rpmsg_char can copy the whole message into an skb and read past the buffer:

  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 the used length, so no subtraction wraps.

This patch is assisted by Claude Opus 5.5.

Fixes 9648224e564a ("rpmsg: validate incoming message length before propagating")

---
v2:
- Leave the check after the dynamic_hex_dump statement.
v1: https://lore.kernel.org/all/[email protected]/

 drivers/rpmsg/virtio_rpmsg_bus.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/rpmsg/virtio_rpmsg_bus.c +0 -0
index 23ff6c8b7a20b..3d58bb19e9297 100644

drivers/rpmsg/virtio_rpmsg_bus.c static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev, · line 679

Quoted 5 lines
drivers/rpmsg/virtio_rpmsg_bus.c +1 -1
@@ -676,7 +676,7 @@ static int rpmsg_recv_single(struct virtproc_info *vrp, struct device *dev,
 	 * the reported payload length.
 	 */
 	if (len > vrp->rx_buf_size ||
-	    msg_len > (len - sizeof(struct rpmsg_hdr))) {
+	    sizeof(struct rpmsg_hdr) + msg_len > len) {

Applied.

Thanks,
Mathieu

Quoted 8 lines
drivers/rpmsg/virtio_rpmsg_bus.c +0 -0
 		dev_warn(dev, "inbound msg too big: (%d, %d)\n", len, msg_len);
 		return -EINVAL;
 	}

base-commit: 6628e7d4c6b25c8d18f1345e3a20d21829aae7e4

Signature · 1 line
2.56.0