All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] rpmsg: virtio: bound the buffer sizes read from config space
@ 2026-10-22 19:41 Lianjie Wang
  2026-10-26 16:59 ` Mathieu Poirier
  2026-11-02 17:42 ` Mathieu Poirier
  0 siblings, 2 replies; 3+ messages in thread
From: Lianjie Wang @ 2026-10-22 19:41 UTC (permalink / raw)
  To: Bjorn Andersson, Mathieu Poirier
  Cc: Lianjie Wang, Tanmay Shah, Arnaud Pouliquen, linux-remoteproc,
	linux-kernel

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.

Fixes: ca96ea25e51a ("rpmsg: virtio_rpmsg_bus: Get buffer size from config space")
Signed-off-by: Lianjie Wang <karin0.zst@gmail.com>
---
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(-)

diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
index 23ff6c8b7a20b..de4af75ee1620 100644
--- a/drivers/rpmsg/virtio_rpmsg_bus.c
+++ b/drivers/rpmsg/virtio_rpmsg_bus.c
@@ -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
-- 
2.56.0


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] rpmsg: virtio: bound the buffer sizes read from config space
  2026-10-22 19:41 [PATCH] rpmsg: virtio: bound the buffer sizes read from config space Lianjie Wang
@ 2026-10-26 16:59 ` Mathieu Poirier
  2026-11-02 17:42 ` Mathieu Poirier
  1 sibling, 0 replies; 3+ messages in thread
From: Mathieu Poirier @ 2026-10-26 16:59 UTC (permalink / raw)
  To: Lianjie Wang
  Cc: Bjorn Andersson, Tanmay Shah, Arnaud Pouliquen, linux-remoteproc,
	linux-kernel

On Thu, 22 Oct 2026 at 13:41, Lianjie Wang <karin0.zst@gmail.com> wrote:
>
> 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.
>
> Fixes: ca96ea25e51a ("rpmsg: virtio_rpmsg_bus: Get buffer size from config space")
> Signed-off-by: Lianjie Wang <karin0.zst@gmail.com>
> ---
> 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(-)
>
> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
> index 23ff6c8b7a20b..de4af75ee1620 100644
> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> @@ -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) {

I will apply this patch when 7.4-rc1 comes out.

Thanks,
Mathieu

>                         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
> --
> 2.56.0
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] rpmsg: virtio: bound the buffer sizes read from config space
  2026-10-22 19:41 [PATCH] rpmsg: virtio: bound the buffer sizes read from config space Lianjie Wang
  2026-10-26 16:59 ` Mathieu Poirier
@ 2026-11-02 17:42 ` Mathieu Poirier
  1 sibling, 0 replies; 3+ messages in thread
From: Mathieu Poirier @ 2026-11-02 17:42 UTC (permalink / raw)
  To: Lianjie Wang
  Cc: Bjorn Andersson, Tanmay Shah, Arnaud Pouliquen, linux-remoteproc,
	linux-kernel

On Fri, Oct 23, 2026 at 04:41:29AM +0900, Lianjie Wang wrote:
> 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.
> 
> Fixes: ca96ea25e51a ("rpmsg: virtio_rpmsg_bus: Get buffer size from config space")
> Signed-off-by: Lianjie Wang <karin0.zst@gmail.com>
> ---
> 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(-)
>

Applied.

Thanks,
Mathieu
 
> diff --git a/drivers/rpmsg/virtio_rpmsg_bus.c b/drivers/rpmsg/virtio_rpmsg_bus.c
> index 23ff6c8b7a20b..de4af75ee1620 100644
> --- a/drivers/rpmsg/virtio_rpmsg_bus.c
> +++ b/drivers/rpmsg/virtio_rpmsg_bus.c
> @@ -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
> -- 
> 2.56.0
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-11-02 17:42 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-22 19:41 [PATCH] rpmsg: virtio: bound the buffer sizes read from config space Lianjie Wang
2026-10-26 16:59 ` Mathieu Poirier
2026-11-02 17:42 ` Mathieu Poirier

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.