rpmsg: virtio: bound the buffer sizes read from config space
PATCH 1/1 rpmsg: virtio: bound the buffer sizes read from config space
With VIRTIO_RPMSG_F_BUFSZ, rpmsg_virtio_get_buf_size() reads the rx and tx buffer sizes from the config space as u32 values and checks only that each can hold the message header. The probe multiplies them by the buffer counts in unsigned int arithmetic to size the buffer area. An rx buffer size of 0x1000000 with 256 rx buffers wraps the rx part to zero, so the allocation covers only the tx buffers, and the driver posts the rx buffers at offsets up to 4 GiB past its start for the device to write.
Reject a buffer larger than the header plus U16_MAX, since the 16-bit msg->len cannot describe a longer payload. With at most 256 buffers in each direction, this bound also keeps the size of the buffer area from wrapping.
This patch is assisted by Claude Opus 5.5.
---
The patch is tested against a virtio-rpmsg device added to QEMU. With buffer sizes in config space that wrap the size of the buffer area, the unpatched driver probes and warns in rpmsg_sg_init() while posting buffers past its allocation. With the patch, the probe fails with "bad vdev config".
drivers/rpmsg/virtio_rpmsg_bus.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
drivers/rpmsg/virtio_rpmsg_bus.c +8 -2
@@ -91,6 +91,7 @@ struct rpmsg_hdr {u8 data[]; } __packed; +#define MAX_RPMSG_BUF_SIZE (sizeof(struct rpmsg_hdr) + U16_MAX)/** * struct virtio_rpmsg_channel - rpmsg channel descriptor
@@ -851,9 +852,14 @@ static int rpmsg_virtio_get_buf_size(struct virtproc_info *vrp) virtio_cread(vdev, struct virtio_rpmsg_config, rxbuf_size, &vrp->tx_buf_size); -/* The buffers must hold at least the rpmsg header */ +/* +* The buffers must hold at least the rpmsg header, and at most +* the header and the largest payload its 16-bit len describes. +*/ if (vrp->rx_buf_size <sizeof (struct rpmsg_hdr ) || - vrp->tx_buf_size <sizeof (struct rpmsg_hdr )) { + vrp->tx_buf_size <sizeof (struct rpmsg_hdr ) || + vrp->rx_buf_size >MAX_RPMSG_BUF_SIZE || + vrp->tx_buf_size >MAX_RPMSG_BUF_SIZE ) {dev_err (&vdev->dev,"bad vdev config: rx buf sz = %u, tx buf sz = %u\n" , vrp->rx_buf_size, vrp->tx_buf_size);
base-commit: bd1ecbb61afefc6a8b876ccbc31e9228b44e9eab
Signature · 1 line
2.56.0
Mathieu Poirier
maintainerQuoted 45 lines · Lianjie Wang
With
VIRTIO_RPMSG_F_BUFSZ,rpmsg_virtio_get_buf_size()reads the rx and tx buffer sizes from the config space as u32 values and checks only that each can hold the message header. The probe multiplies them by the buffer counts in unsigned int arithmetic to size the buffer area. An rx buffer size of0x1000000with 256 rx buffers wraps the rx part to zero, so the allocation covers only the tx buffers, and the driver posts the rx buffers at offsets up to 4 GiB past its start for the device to write.Reject a buffer larger than the header plus
U16_MAX, since the 16-bitmsg->lencannot describe a longer payload. With at most 256 buffers in each direction, this bound also keeps the size of the buffer area from wrapping.This patch is assisted by Claude Opus 5.5.
Fixes ca96ea25e51a ("rpmsg: virtio_rpmsg_bus: Get buffer size from config space")Signed-off-by Lianjie Wang <[email protected]>---
The patch is tested against a virtio-rpmsg device added to QEMU. With buffer sizes in config space that wrap the size of the buffer area, the unpatched driver probes and warns inrpmsg_sg_init()while posting buffers past its allocation. With the patch, the probe fails with "bad vdev config".drivers/rpmsg/virtio_rpmsg_bus.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)drivers/rpmsg/virtio_rpmsg_bus.c +1
-0@@ -91,6 +91,7 @@ struct rpmsg_hdr {u8 data[]; } __packed; +#define MAX_RPMSG_BUF_SIZE (sizeof(struct rpmsg_hdr) + U16_MAX)/** * struct virtio_rpmsg_channel - rpmsg channel descriptor @@ -851,9 +852,14 @@ static int rpmsg_virtio_get_buf_size(struct virtproc_info *vrp) virtio_cread(vdev, struct virtio_rpmsg_config, rxbuf_size, &vrp->tx_buf_size);
drivers/rpmsg/virtio_rpmsg_bus.c static int rpmsg_virtio_get_buf_size(struct virtproc_info *vrp) · line 862
Quoted 10 lines
drivers/rpmsg/virtio_rpmsg_bus.c +7
-2-/* The buffers must hold at least the rpmsg header */ +/* +* The buffers must hold at least the rpmsg header, and at most +* the header and the largest payload its 16-bit len describes. +*/ if (vrp->rx_buf_size <sizeof (struct rpmsg_hdr ) || - vrp->tx_buf_size <sizeof (struct rpmsg_hdr )) { + vrp->tx_buf_size <sizeof (struct rpmsg_hdr ) || + vrp->rx_buf_size >MAX_RPMSG_BUF_SIZE || + vrp->tx_buf_size >MAX_RPMSG_BUF_SIZE ) {
I will apply this patch when 7.4-rc1 comes out.
Thanks,
Mathieu
Quoted 8 lines
drivers/rpmsg/virtio_rpmsg_bus.c +0
-0dev_err (&vdev->dev,"bad vdev config: rx buf sz = %u, tx buf sz = %u\n" , vrp->rx_buf_size, vrp->tx_buf_size);base-commit:
bd1ecbb61afefc6a8b876ccbc31e9228b44e9eab--
2.56.0
Quoted 25 lines · Lianjie Wang
With
VIRTIO_RPMSG_F_BUFSZ,rpmsg_virtio_get_buf_size()reads the rx and tx buffer sizes from the config space as u32 values and checks only that each can hold the message header. The probe multiplies them by the buffer counts in unsigned int arithmetic to size the buffer area. An rx buffer size of0x1000000with 256 rx buffers wraps the rx part to zero, so the allocation covers only the tx buffers, and the driver posts the rx buffers at offsets up to 4 GiB past its start for the device to write.Reject a buffer larger than the header plus
U16_MAX, since the 16-bitmsg->lencannot describe a longer payload. With at most 256 buffers in each direction, this bound also keeps the size of the buffer area from wrapping.This patch is assisted by Claude Opus 5.5.
Fixes ca96ea25e51a ("rpmsg: virtio_rpmsg_bus: Get buffer size from config space")Signed-off-by Lianjie Wang <[email protected]>---
The patch is tested against a virtio-rpmsg device added to QEMU. With buffer sizes in config space that wrap the size of the buffer area, the unpatched driver probes and warns inrpmsg_sg_init()while posting buffers past its allocation. With the patch, the probe fails with "bad vdev config".
Quoted 3 lines
drivers/rpmsg/virtio_rpmsg_bus.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)
Applied.
Thanks,
Mathieu
Quoted 34 lines
drivers/rpmsg/virtio_rpmsg_bus.c +8
-2@@ -91,6 +91,7 @@ struct rpmsg_hdr {u8 data[]; } __packed; +#define MAX_RPMSG_BUF_SIZE (sizeof(struct rpmsg_hdr) + U16_MAX)/** * struct virtio_rpmsg_channel - rpmsg channel descriptor @@ -851,9 +852,14 @@ static int rpmsg_virtio_get_buf_size(struct virtproc_info *vrp) virtio_cread(vdev, struct virtio_rpmsg_config, rxbuf_size, &vrp->tx_buf_size); -/* The buffers must hold at least the rpmsg header */ +/* +* The buffers must hold at least the rpmsg header, and at most +* the header and the largest payload its 16-bit len describes. +*/ if (vrp->rx_buf_size <sizeof (struct rpmsg_hdr ) || - vrp->tx_buf_size <sizeof (struct rpmsg_hdr )) { + vrp->tx_buf_size <sizeof (struct rpmsg_hdr ) || + vrp->rx_buf_size >MAX_RPMSG_BUF_SIZE || + vrp->tx_buf_size >MAX_RPMSG_BUF_SIZE ) {dev_err (&vdev->dev,"bad vdev config: rx buf sz = %u, tx buf sz = %u\n" , vrp->rx_buf_size, vrp->tx_buf_size);base-commit:
bd1ecbb61afefc6a8b876ccbc31e9228b44e9eabSignature · 1 line
2.56.0