Re: [PATCH net-next v5 3/3] net: stmmac: allow non-linear xdp_buff in XDP mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] net: stmmac: AF_XDP Zero-Copy RX path data corruption with jumbo frames
- [High] net: stmmac: truncated jumbo frames passed to XDP ZC
- [Medium] net/stmmac: silent drop of jumbo frames with non-SG XDP
--

commit ea7721d2338075cf38b4bf936b99f96440c2eceb
Author: Lorenzo Bianconi <lorenzo.bianconi@oss.qualcomm.com>

net: stmmac: allow non-linear xdp_buff in XDP mode

This commit drops the checks that rejected attaching an XDP program when
the MTU is larger than ETH_DATA_LEN, and that refused to increase the MTU
while XDP is enabled. This allows XDP to operate on frames bigger than
ETH_DATA_LEN since the RX path now builds multi-buffer xdp_buffs.

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -6351,12 +6351,6 @@ static int stmmac_change_mtu(struct net_device *dev, int new_mtu)
>  		txfifosz = priv->dma_cap.tx_fifo_size;
>  
>  	txfifosz /= priv->plat->tx_queues_to_use;
> -
> -	if (stmmac_xdp_is_enabled(priv) && new_mtu > ETH_DATA_LEN) {
> -		netdev_dbg(priv->dev, "Jumbo frames not supported for XDP\n");
> -		return -EINVAL;
> -	}
> -
>  	new_mtu = STMMAC_ALIGN(new_mtu);

[Severity: Critical]
Does removing this MTU restriction expose the AF_XDP Zero-Copy RX path to
jumbo frames without multi-buffer support?

In stmmac_rx_zc(), when a jumbo frame arrives and spans multiple RX
descriptors, intermediate segments (where rx_not_ls is set) are dropped:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:stmmac_rx_zc() {
    ...
    /* XSK pool expects RX frame 1:1 mapped to XSK buffer */
    if (likely(status & rx_not_ls)) {
        xsk_buff_free(buf->xdp);
        buf->xdp = NULL;
        dirty++;
        count++;
        goto read_again;
    }
    ...
}

However, the error flag is never set when dropping these intermediate
segments. When the final segment arrives (rx_not_ls is 0), it is processed
as a brand-new, valid packet because error is 0. 

Since len was never accumulated, the segment length is calculated based on
the descriptor's total packet length. This artificially expands the XDP
buffer's data_end to encompass the full frame size, even though the hardware
only wrote the tail of the jumbo frame into this buffer.

Could this pass the tail of the jumbo frame, followed by uninitialized
user-space memory from the XSK pool, to the XDP program and network stack,
resulting in data corruption or an information leak?

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
> @@ -109,14 +109,6 @@ int stmmac_xdp_set_prog(struct stmmac_priv *priv, struct bpf_prog *prog,
>  
>  	if_running = netif_running(dev);
>  
> -	if (prog && dev->mtu > ETH_DATA_LEN) {
> -		/* For now, the driver doesn't support XDP functionality with
> -		 * jumbo frames so we return error.
> -		 */
> -		NL_SET_ERR_MSG_MOD(extack, "Jumbo frames not supported");
> -		return -EOPNOTSUPP;
> -	}
> -
>  	if (!prog)
>  		xdp_features_clear_redirect_target(dev);

[Severity: Medium]
Should this check have been replaced with a validation against
prog->aux->xdp_has_frags rather than being removed entirely?

Removing this unconditionally allows a user to configure an MTU > 1500 while
a single-buffer XDP program is already loaded.

If a non-SG XDP program is attached and a jumbo frame arrives,
stmmac_xdp_run_prog() bypasses __stmmac_xdp_run_prog() for multi-buffer
packets:

drivers/net/ethernet/stmicro/stmmac/stmmac_main.c:stmmac_xdp_run_prog() {
    int res = STMMAC_XDP_CONSUMED;
    ...
    if (likely(!xdp_buff_has_frags(xdp) || prog->aux->xdp_has_frags))
        res = __stmmac_xdp_run_prog(priv, prog, xdp);
out:
    return ERR_PTR(-res);
}

Will this result in all jumbo traffic being silently dropped without warning
when a non-SG XDP program is attached?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-stmmac-rx-mb-v5-0-c38fa4eaa138@oss.qualcomm.com?part=3