Re: [PATCH net 3/3] net: enetc: let core segment GSO super-skb when headers exceed TSO_HEADER_SIZE

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] net: enetc: VLAN tag push after enetc_features_check()
  overflows TSO header
- [Medium] net: enetc: u8 hdr_len truncation in enetc_lso_start() for
  256-byte headers
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [High] net: enetc: lockup in enetc_map_tx_tso_hdr() via undersized TSO
  descriptor [1]
--

commit b5eb34f7b5b23a1224c351b91a143ecbd34866d9
Author: Vladimir Oltean <vladimir.oltean@nxp.com>

net: enetc: let core segment GSO super-skb when headers exceed TSO_HEADER_SIZE

This commit clears NETIF_F_GSO_MASK in enetc_features_check() when the GSO
packet headers exceed TSO_HEADER_SIZE so the networking core segments the
packet instead of overflowing the per-ring TSO header slot.

> diff --git a/drivers/net/ethernet/freescale/enetc/enetc.c b/drivers/net/ethernet/freescale/enetc/enetc.c
> index 7f089a84d9a65..3c5478238e356 100644
> --- a/drivers/net/ethernet/freescale/enetc/enetc.c
> +++ b/drivers/net/ethernet/freescale/enetc/enetc.c
> @@ -1122,7 +1122,15 @@ netdev_features_t enetc_features_check(struct sk_buff *skb,
>  
>  	features = vlan_features_check(skb, features);
>  
> -	if (!skb_is_gso(skb) || enetc_skb_uses_lso(priv, skb))
> +	if (!skb_is_gso(skb))
> +		return features;
> +
> +	if (enetc_lso_get_hdr_len(skb) > TSO_HEADER_SIZE) {
> +		features &= ~NETIF_F_GSO_MASK;
> +		return features;
> +	}

[Severity: High]
Can a non-offloaded VLAN tag push the header past TSO_HEADER_SIZE after
this check?

In validate_xmit_skb(), netif_skb_features() runs before
validate_xmit_vlan().  When a GSO skb carries an out-of-band VLAN tag that
is not offloaded in hardware (such as an 802.1ad S-TAG, or an 802.1Q C-TAG
when NETIF_F_HW_VLAN_CTAG_TX is disabled), a 253..256 byte header passes
enetc_features_check() and then grows by VLAN_HLEN (4 bytes) when
validate_xmit_vlan() calls __vlan_hwaccel_push_inside().  In
enetc_map_tx_tso_buffs(), tso_build_hdr() then copies 257..260 bytes into
a 256-byte slot in tx_ring->tso_headers, overflowing into the next slot or
past the end of the coherent DMA buffer.

[Severity: Medium]
Can a 256-byte header still break the hardware LSO path because
lso->hdr_len is a u8?

Because this check uses > TSO_HEADER_SIZE (256), a GSO packet whose headers
are 256 bytes passes through to enetc_lso_start():

drivers/net/ethernet/freescale/enetc/enetc.h:
struct enetc_lso_t {
	...
	u8	hdr_len; /* LSO header length */

drivers/net/ethernet/freescale/enetc/enetc.c:enetc_lso_start() {
	...
	lso->hdr_len = enetc_lso_get_hdr_len(skb);
	lso->total_len = skb->len - lso->hdr_len;
}

Assigning 256 to lso->hdr_len truncates it to 0, so enetc_lso_map_hdr()
copies 0 header bytes and programs hdr_len = 0 in the LSO descriptor while
enetc_lso_map_data() maps the headers as payload.

[Severity: High]
This problem wasn't introduced by this patch, but in [1] can a GSO
packet with hdr_len < 16 lock up the controller in enetc_map_tx_tso_hdr()?

While this check bounds the maximum header length, a crafted GSO packet
(for example via AF_PACKET) with hdr_len < 16 still reaches
enetc_map_tx_tso_hdr(), which writes hdr_len directly into
txbd_tmp.buf_len.  An undersized first TSO descriptor below the hardware's
16-byte minimum stalls the ENETC transmitter.

[1] https://sashiko.dev/#/bug/linux-835a7475-1686-40c3-bf3a-572547fbf43f

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111233149.479596-1-vladimir.oltean@nxp.com?part=3