[PATCH] remoteproc: virtio: fix uninitialized buffer use in rproc_virtio_get()

[PATCH] remoteproc: virtio: fix uninitialized buffer use in rproc_virtio_get()

[Kery Qi]

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 <qikeyu2017@gmail.com>
---
 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];
 
-	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);
+
+	if (offset > rsc->config_len)
 		return;
-	}
+
+		if (len > rsc->config_len - offset)
+			len = rsc->config_len - offset;
 
 	memcpy(buf, cfg + offset, len);
 }
-- 
2.34.1

Re: [PATCH] remoteproc: virtio: fix uninitialized buffer use in rproc_virtio_get()

[Zhongqiu Han]

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 <qikeyu2017@gmail.com>

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

Re: [PATCH] remoteproc: virtio: fix uninitialized buffer use in rproc_virtio_get()

[Mathieu Poirier]

Good day Kery,

On Fri, Jan 09, 2026 at 03:23:36AM +0800, 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 <qikeyu2017@gmail.com>
> ---
>  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];
>  
> -	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);

I think this should be done in the caller, not virtio_config_ops->get().  I am
willing to reconsider if you include this change in a patchset that changes all
the other virtio_config_ops->get() function.

Thanks,
Mathieu

> +
> +	if (offset > rsc->config_len)
>  		return;
> -	}
> +
> +		if (len > rsc->config_len - offset)
> +			len = rsc->config_len - offset;
>  
>  	memcpy(buf, cfg + offset, len);
>  }
> -- 
> 2.34.1
> 
>