Re: [PATCH net-next v5 01/12] gve: add struct gve_device_info to hold device properties

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver trusts the device-provided `descriptor->total_length` without clamping it to the actual allocated DMA buffer size, leading to an out-of-bounds read vulnerability.
--

commit c3056c70d520a5fcb8777d7ecaee3b31bf27e333
Author: Harshitha Ramamurthy <hramamurthy@google.com>

gve: add struct gve_device_info to hold device properties

The commit refactors the initialization flow by adding struct gve_device_info
to store negotiated device properties. It fills it in
gve_adminq_describe_device() and its callees to prepare the driver for the
upcoming MailboxQ mode without changing functional behavior.

> diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
> index f05f4895f4c74..512349c5517f4 100644
> --- a/drivers/net/ethernet/google/gve/gve_adminq.c
> +++ b/drivers/net/ethernet/google/gve/gve_adminq.c
> @@ -70,7 +70,7 @@ void gve_parse_device_option(struct gve_priv *priv,
>  
>  		dev_info(&priv->pdev->dev,
>  			 "Gqi raw addressing device option enabled.\n");
> -		priv->queue_format = GVE_GQI_RDA_FORMAT;
> +		priv->device_info.queue_format = GVE_GQI_RDA_FORMAT;
>  		break;

[Severity: High]
This isn't a bug introduced by this patch, but I noticed a potential
out-of-bounds read vulnerability in the device option parsing logic.

When traversing the options in gve_process_device_options(), the code relies on
gve_get_next_option() which does not clamp the untrusted total_length to the
allocated buffer size:

drivers/net/ethernet/google/gve/gve_adminq.c:gve_get_next_option() {
	void *option_end, *descriptor_end;

	option_end = (void *)(option + 1) + be16_to_cpu(option->option_length);
	descriptor_end = (void *)descriptor + be16_to_cpu(descriptor->total_length);

	return option_end > descriptor_end ? NULL : (struct gve_device_option *)option_end;
}

If a malicious device provides an AdminQ response where
descriptor->total_length exceeds GVE_ADMINQ_BUFFER_SIZE (4096),
descriptor_end will point far beyond the actual allocated DMA buffer.

This could allow gve_get_next_option() to return an out-of-bounds pointer,
which is then dereferenced in the option parsing loop in
gve_process_device_options():

drivers/net/ethernet/google/gve/gve_adminq.c:gve_process_device_options() {
	...
	for (i = 0; i < num_options; i++) {
		struct gve_device_option *next_opt;

		next_opt = gve_get_next_option(descriptor, dev_opt);
	...
}

Could descriptor->total_length be validated against GVE_ADMINQ_BUFFER_SIZE
before using it to calculate descriptor_end?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903215606.31633-1-hramamurthy@google.com?part=1