Re: [PATCH net 3/4] gve: Use default min ring size when device option values are 0

[Pin-yen Lin]

On Mon, Apr 20, 2026 at 10:18 AM Harshitha Ramamurthy
<hramamurthy@xxxxxxxxxx> wrote:
>
> From: Pin-yen Lin <treapking@xxxxxxxxxx>
>
> On gvnic devices that support reporting minimum ring sizes, the device
> option always includes the min_(rx|tx)_ring_size fields, and the values
> will be 0 if they are not configured to be exposed. This makes the
> driver allow unexpected small ring size configurations from the
> userspace.
>
> Use the default ring size in the driver if the min ring sizes from the
> device option are 0.
>
> This was discovered by drivers/net/ring_reconfig.py selftest.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: ed4fb326947d ("gve: add support to read ring size ranges from the device")
> Reviewed-by: Joshua Washington <joshwash@xxxxxxxxxx>
> Reviewed-by: Jordan Rhee <jordanrhee@xxxxxxxxxx>
> Signed-off-by: Pin-yen Lin <treapking@xxxxxxxxxx>
> Signed-off-by: Harshitha Ramamurthy <hramamurthy@xxxxxxxxxx>
> ---
>  drivers/net/ethernet/google/gve/gve_adminq.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/google/gve/gve_adminq.c b/drivers/net/ethernet/google/gve/gve_adminq.c
> index b72cc0fa2ba2..57d898f6fa82 100644
> --- a/drivers/net/ethernet/google/gve/gve_adminq.c
> +++ b/drivers/net/ethernet/google/gve/gve_adminq.c
> @@ -189,7 +189,9 @@ void gve_parse_device_option(struct gve_priv *priv,
>                 *dev_op_modify_ring = (void *)(option + 1);
>
>                 /* device has not provided min ring size */
> -               if (option_length == GVE_DEVICE_OPTION_NO_MIN_RING_SIZE)
> +               if (option_length == GVE_DEVICE_OPTION_NO_MIN_RING_SIZE ||
> +                   be16_to_cpu((*dev_op_modify_ring)->min_rx_ring_size) == 0 ||
> +                   be16_to_cpu((*dev_op_modify_ring)->min_tx_ring_size) == 0)

Sashiko says:

Does this introduce an out-of-bounds read regression if the option length
is between 9 and 11?
The length check earlier in the switch case only ensures option_length is at
least GVE_DEVICE_OPTION_NO_MIN_RING_SIZE (which is 8). The new condition
reads min_tx_ring_size, which sits at bytes 10-11 of the structure.
If an option length of 10 is provided, the condition would read past the
provided data length. Could this trigger a page fault if the option payload
happens to be placed at the very end of an unmapped DMA page boundary?
Could this introduce a time-of-check to time-of-use regression? Can these
values be modified by the device between this check and when they are used
later?
Since dev_op_modify_ring points into shared DMA memory, the values are
fetched here for validation. If the device alters the DMA memory to 0 after
this check has completed, would the driver bypass this validation when
applying the values later, leading to a minimum ring size of 0?
Does this introduce a logic regression by discarding a valid configuration
for one ring if the other ring's size is 0?
For example, if min_rx_ring_size is 0 but min_tx_ring_size is explicitly
configured to a valid size like 1024, setting a single default_min_ring_size
flag seems to force both rings to use their default fallback values instead
of evaluating their validity independently.

This should not be an issue because the GVNIC never returns an option
length between 9 and 11. Depending on the GVNIC version, it either
omits the ring sizes or includes both RX and TX ring sizes (even if
the values are 0).

>                         priv->default_min_ring_size = true;
>                 break;
>         case GVE_DEV_OPT_ID_FLOW_STEERING:
> --
> 2.54.0.rc0.605.g598a273b03-goog
>

Regards,
Pin-yen