Re: [PATCH net-next v3 2/2] gve: add XDP metadata support for DQ RDA

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] In GVE DQO QPL mode with XDP enabled, `buf_state->page_info.pad` accumulates XDP head adjustments and drifts unbounded, leading to out-of-bounds hardware DMA.
- [High] Missing fragment drop tracking for multi-descriptor packets in the DQO RX path causes subsequent fragments of a dropped packet to be treated as new packets, bypassing XDP and network security filters.
--

--- Patch [2]: [PATCH net-next v3 2/2] gve: add XDP metadata support for DQ RDA ---
commit 7fa79d59c32cc58b133c8b5735d2691860bcdca2
Author: Joshua Washington <joshwash@google.com>

gve: add XDP metadata support for DQ RDA

This commit updates xdp_prepare_buff to set meta_valid to true in
gve_rx_dqo. This enables XDP programs to use metadata in the DQ RDA mode.

> diff --git a/drivers/net/ethernet/google/gve/gve_rx_dqo.c b/drivers/net/ethernet/google/gve/gve_rx_dqo.c
> --- a/drivers/net/ethernet/google/gve/gve_rx_dqo.c
> +++ b/drivers/net/ethernet/google/gve/gve_rx_dqo.c
[ ... ]
>  	/* Append to current skb if one exists. */
>  	if (rx->ctx.skb_head) {
>  		if (unlikely(gve_rx_append_frags(napi, buf_state, buf_len, rx,
>  						 priv)) != 0) {
>  			goto error;
>  		}
>  		return 0;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but I have a question about
fragment drop tracking for multi-descriptor packets in this DQO RX path.

If the first fragment of a multi-descriptor packet is dropped (e.g., by XDP),
rx->ctx.skb_head remains NULL. Unlike the GQI path which tracks
rx->ctx.drop_pkt, the DQO path does not seem to track dropped state.

Will subsequent fragments of that dropped packet see rx->ctx.skb_head == NULL,
bypass this append logic, and fall through to the XDP block below, treating
the fragment as a new packet?

>  
>  	if (xprog) {
>  		struct gve_xdp_buff gve_xdp;
>  		void *old_data;
>  		int xdp_act;
>  
>  		xdp_init_buff(&gve_xdp.xdp, buf_state->page_info.buf_size,
>  			      &rx->xdp_rxq);
>  		xdp_prepare_buff(&gve_xdp.xdp,
>  				 buf_state->page_info.page_address +
>  				 buf_state->page_info.page_offset,
>  				 buf_state->page_info.pad,
> -				 buf_len, false);
> +				 buf_len, true);
>  		gve_xdp.gve = priv;
>  		gve_xdp.compl_desc = compl_desc;
>  
>  		old_data = gve_xdp.xdp.data;
>  		xdp_act = bpf_prog_run_xdp(xprog, &gve_xdp.xdp);
>  		buf_state->page_info.pad += gve_xdp.xdp.data - old_data;

[Severity: High]
This is a pre-existing issue, but does accumulating the XDP head adjustment
into buf_state->page_info.pad lead to out-of-bounds hardware DMA?

When XDP programs adjust the packet head, the difference is accumulated into
the pad. When the buffer is recycled, it appears the pad is never reset to
the default headroom. 

Could this cause subsequent reuses of the buffer to continue accumulating
the offset and eventually calculate an out-of-bounds DMA address in
gve_alloc_buffer():

drivers/net/ethernet/google/gve/gve_buffer_mgmt_dqo.c:gve_alloc_buffer() {
    ...
    desc->buf_addr = cpu_to_le64(buf_state->addr +
                                 buf_state->page_info.page_offset +
                                 buf_state->page_info.pad);
    ...
}

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