Re: [PATCH net 6/6] xsk: validate metadata when processing requests

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Zero-copy drivers unconditionally parse XDP TX metadata ignoring the XDP_TX_METADATA option flag.
- [High] The stmmac driver drops AF_XDP TX frames without generating completions when the frame length exceeds max_sdu, causing the AF_XDP completion queue (CQ) to permanently desynchronize.
--

--- Patch [6]: [PATCH net 6/6] xsk: validate metadata when processing requests ---
commit 8967b4032038ba439c0a13ce2dc4cab303b87c2e
Author: Stanislav Fomichev <sdf.kernel@gmail.com>

xsk: validate metadata when processing requests
    
The zero-copy path validates TX metadata while obtaining the descriptor
context, then reads it again later when preparing the hardware request.
Validate the metadata in xsk_tx_metadata_request() and use the resulting
flags snapshot for every feature check.

> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index 5a18d00deb4f4..7b6cdc037c00f 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -3082,7 +3082,7 @@ static void igc_xdp_xmit_zc(struct igc_ring *ring)
>  		meta_req.tx_buffer = bi;
>  		meta_req.meta = meta;
>  		meta_req.used_desc = 0;
> -		xsk_tx_metadata_request(&meta, &igc_xsk_tx_metadata_ops,
> +		xsk_tx_metadata_request(pool, &meta, &igc_xsk_tx_metadata_ops,
>  					&meta_req);

[Severity: High]
This is a pre-existing issue, but do the zero-copy drivers unconditionally
parse XDP TX metadata while ignoring the XDP_TX_METADATA option flag?

Looking at igc_xdp_xmit_zc() (and similarly in mlx5e_xsk_tx() and
stmmac_xdp_xmit_zc()), the zero-copy path reads metadata directly without
checking xdp_desc.options:

    meta = xsk_buff_get_metadata(pool, xdp_desc.addr);

If an application sends a standard AF_XDP packet without requesting metadata
(by clearing the XDP_TX_METADATA flag), the driver ignores desc->options and
unconditionally reads the metadata memory. Since the memory might contain
stale data, meta->flags could contain valid but bogus bits.

Could this trigger unintended hardware offloads, causing data corruption
(bogus checksum offsets) or TX queue stalls (bogus launch times)?

[ ... ]

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 3f5111969c414..eb666b0bcdd22 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2747,8 +2747,8 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
>  		meta_req.set_ic = &set_ic;
>  		meta_req.tbs = tx_q->tbs;
>  		meta_req.edesc = &tx_q->dma_entx[entry];
> -		xsk_tx_metadata_request(&meta, &stmmac_xsk_tx_metadata_ops,
> -					&meta_req);
> +		xsk_tx_metadata_request(pool, &meta,
> +					&stmmac_xsk_tx_metadata_ops, &meta_req);

[Severity: High]
This isn't a bug introduced by this patch, but does stmmac_xdp_xmit_zc()
permanently desynchronize the AF_XDP completion queue when dropping oversized
frames?

Earlier in stmmac_xdp_xmit_zc(), if the frame length exceeds max_sdu, the
descriptor is dropped via continue without issuing an XSK completion or
posting a dummy hardware descriptor:

    if (priv->est && priv->est->enable &&
        priv->est->max_sdu[queue] &&
        xdp_desc.len > priv->est->max_sdu[queue]) {
        priv->xstats.max_sdu_txq_drop[queue]++;
        continue;
    }

Since xsk_tx_peek_desc() reserves a slot in the AF_XDP completion queue (CQ),
dropping the frame skips the hardware descriptor setup, and no completion
interrupt is generated for this packet.

When a subsequent packet completes, xsk_tx_completed() will advance the CQ
producer. Because CQ entries are positional, would user space receive the
address of the dropped packet as a completion instead of the actually
completed packet, permanently leaking descriptors?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727161959.885642-1-sdf@fomichev.me?part=6