On 1/9/2026 3:23 AM, Kery Qi wrote:
rproc_virtio_get() returns early on out-of-bounds access without touching
the caller-provided buffer. Callers of virtio config accessors commonly
pass stack storage and do not get an error code back, so stale/uninit
bytes may be consumed as device configuration, leading to unpredictable
behavior and potentially leaking stack data if later exposed.
Always clear the destination buffer, reject offsets past config_len, and
clamp the read length to the available config bytes before copying.
Fixes: 92b38f851470 ("remoteproc: support virtio config space.")
Signed-off-by: Kery Qi <[email protected]>
Please use ./scripts/get_maintainer.pl to retrieve the full list of
maintainer email and cc list.
---
drivers/remoteproc/remoteproc_virtio.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/remoteproc/remoteproc_virtio.c
b/drivers/remoteproc/remoteproc_virtio.c
index c5d46a878149..8fa8c8a86b4b 100644
--- a/drivers/remoteproc/remoteproc_virtio.c
+++ b/drivers/remoteproc/remoteproc_virtio.c
@@ -298,10 +298,13 @@ static void rproc_virtio_get(struct virtio_device *vdev,
unsigned int offset,
rsc = (void *)rvdev->rproc->table_ptr + rvdev->rsc_offset;
cfg = &rsc->vring[rsc->num_of_vrings];
Hi Kery,
You want to initialize the buffer to avoid issues caused by calling the
virtio_cread macro with an uninitialized variable buffer, which should
be fair enough,
virtio_cread(vdev, structname, member, ptr)
vdev->config->get()
OR
__virtio_cread_many()
vdev->config->get()
- if (offset + len > rsc->config_len || offset + len < len) {
- dev_err(&vdev->dev, "rproc_virtio_get: access out of bounds\n");
+ memset(buf, 0, len);
+
The original check offset + len > rsc->config_len ensures that access to
the config space does not overflow, while offset + len < len is used to
detect unsigned integer overflow.
+ if (offset > rsc->config_len)
Any log info?
return;
- }
And then may I know what does this really mean? Are you suggesting a
quick fast path check or something else? Please clarify the reasoning
behind this, and if that makes sense, please split the initialization
and the boundary check into two separate patches.
+
+ if (len > rsc->config_len - offset)
Formatting issue here.
+ len = rsc->config_len - offset;
Why is truncation allowed here? Any err log info?
memcpy(buf, cfg + offset, len);
}
--
Thx and BRs,
Zhongqiu Han
