methodmissing commented on code in PR #20538: URL: https://github.com/apache/nuttx/pull/20538#discussion_r4228029092
########## openamp/0020-rpmsg_virtio-do-not-trust-lengths-written-by-the-remote.patch: ########## @@ -0,0 +1,62 @@ +From 6710e18a7b9631f5da2bdc8010ba0a79ac0e4317 Mon Sep 17 00:00:00 2001 +From: Lourens Naude <[email protected]> +Date: Thu, 8 Oct 2026 22:14:33 +0100 +Subject: [PATCH 20/20] rpmsg_virtio: do not trust lengths written by the + remote + +On the driver side every TX buffer is h2r_buf_size long and every RX +buffer r2h_buf_size long; used->len and rpmsg_hdr.len come from the +remote. A used->len of 0 turned into a 4 GB payload capacity, a header +length larger than the buffer let endpoint callbacks read past it. + +Signed-off-by: Lourens Naude <[email protected]> +--- + lib/rpmsg/rpmsg_virtio.c | 16 ++++++++++++++-- + 1 file changed, 14 insertions(+), 2 deletions(-) + +diff --git a/lib/rpmsg/rpmsg_virtio.c open-amp/lib/rpmsg/rpmsg_virtio.c +index 9913d87..45783ab 100644 +--- a/lib/rpmsg/rpmsg_virtio.c ++++ open-amp/lib/rpmsg/rpmsg_virtio.c +@@ -198,9 +198,11 @@ static void *rpmsg_virtio_get_tx_buffer(struct rpmsg_virtio_device *rvdev, + if (!data && rvdev->svq->vq_free_cnt) { + data = rpmsg_virtio_shm_pool_get_buffer(rvdev->shpool, + rvdev->config.h2r_buf_size); +- *len = rvdev->config.h2r_buf_size; + *idx = 0; + } ++ /* used->len was written by the remote; the buffer is ours */ ++ if (data) ++ *len = rvdev->config.h2r_buf_size; Review Comment: Neither: the 0 was injected on purpose, the window is uncached and barriered. Virtio 1.2 §2.7.8 defines `used->len` as “the number of bytes written into the device writable portion of the buffer” (https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html#x1-430008), which is 0 for a TX buffer, **so a conforming remote breaks the host too. The host knows its own buffer size and should not read it back from the peer.** ```c struct virtq_used { #define VIRTQ_USED_F_NO_NOTIFY 1 le16 flags; le16 idx; struct virtq_used_elem ring[ /* Queue Size */]; le16 avail_event; /* Only if VIRTIO_F_EVENT_IDX */ }; /* le32 is used here for ids for padding reasons. */ struct virtq_used_elem { /* Index of start of used descriptor chain. */ le32 id; /* * The number of bytes written into the device writable portion of * the buffer described by the descriptor chain. */ le32 len; }; ``` * https://github.com/PX4/PX4-Autopilot/pull/28880/changes#diff-2d9fe79e7dd90b989e2bc26d502a6e6cc28503bce16c76950cdcabcd7b06b2efR68-R72 * https://github.com/PX4/PX4-Autopilot/pull/28880/changes#diff-886e0b6b8b03929efe46e9872bec565ce75c3d940a27bb95a6dd40313371086fR50-R57 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
