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

[Pin-yen Lin]

On Fri, Apr 24, 2026 at 5:25 PM 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 08587bf40ed4..2cd0dd6ced94 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:

Could this cause an out-of-bounds memory read when processing malformed
device option lengths?
Looking at the earlier validation in gve_parse_device_option() for
GVE_DEV_OPT_ID_MODIFY_RING, the length is only verified to be at least
GVE_DEVICE_OPTION_NO_MIN_RING_SIZE (8 bytes), while the full
struct gve_device_option_modify_ring is 12 bytes:
    if (option_length < GVE_DEVICE_OPTION_NO_MIN_RING_SIZE ||
        req_feat_mask != GVE_DEV_OPT_REQ_FEAT_MASK_MODIFY_RING) {
If the device provides an option_length of 9, 10, or 11, the initial
option_length == 8 check will evaluate to false. The code will then
evaluate the next conditions and read min_rx_ring_size and
min_tx_ring_size.
Since these fields are located at offsets 8-11, this reads past the end
of the provided option payload.
Because the descriptor buffer is allocated via a page-aligned dma_pool_alloc
of exactly GVE_ADMINQ_BUFFER_SIZE, if the malformed option ends exactly at
the page boundary, could this read cross into adjacent, potentially unmapped
memory and result in a page fault?
Would it be safer to verify that option_length is large enough to contain
the minimum ring size fields before dereferencing them?

GVNIC will never return an option length of 9, 10, or 11, so this
shouldn't be an issue.
>                         priv->default_min_ring_size = true;
>                 break;
>         case GVE_DEV_OPT_ID_FLOW_STEERING:
> --
> 2.54.0.545.g6539524ca2-goog
>

Regards,
Pin-yen