Re: [PATCH net v2] net/mlx5e: SHAMPO, Fix IP length overflow on large HW GRO sessions

[Dragos Tatulea]

On 17.08.26 20:16, Tianyu Zuo wrote:
> mlx5e_hw_gro_skb_has_enough_space() bounds a HW GRO session by the
> payload held in the skb fragments only. The L3/L4 headers that
> header-data split placed in the linear area are not accounted for, and
> the limit is inclusive of GRO_LEGACY_MAX_SIZE.
> 
> On a 4K page system a session can therefore grow to 16 full page
> fragments (65536 bytes) plus the 40 bytes of IPv4/TCP headers in the
> linear part, giving skb->len = 65576.
> 
> mlx5e_shampo_update_hdr() writes the IP length itself:
> 
> 	__be16 newlen = htons(skb->len - nhoff);
> 	csum_replace2(&ipv4->check, ipv4->tot_len, newlen);
> 	ipv4->tot_len = newlen;
> 
> The result wraps. ip_rcv_core() then trims the 64KB skb down to the
> wrapped tot_len, silently dropping the payload. The IPv6 path wraps
> identically in ipv6hdr->payload_len.
> 
> Both branches of the check are inclusive of GRO_LEGACY_MAX_SIZE, so a
> session can reach skb->len == 65536 and wrap tot_len to zero. In the
> page_size >= GRO_LEGACY_MAX_SIZE branch skb->len already covers the
> linear area, so that off by one is the only problem there. The fragment
> based branch additionally omits the linear area entirely.
> 
> Account for skb_headlen() in the fragment based branch and make both
> comparisons strictly less than GRO_LEGACY_MAX_SIZE, so that skb->len can
> never exceed 65535. The new bound is strictly tighter than the old one,
> so the implicit limit on the fragment count (at most 65536 / page_size,
> well below MAX_SKB_FRAGS) is preserved.
> 
> Fixes: 92552d3abd32 ("net/mlx5e: HW_GRO cqe handler implementation")
> Signed-off-by: Tianyu Zuo <cosmosocket@xxxxxxxxx>
> ---
> v2: commit message only, per review from Dragos and Tariq.
> 
> v1: https://lore.kernel.org/netdev/20260729204745.166584-1-cosmosocket@xxxxxxxxx/
> 
>  drivers/net/ethernet/mellanox/mlx5/core/en_rx.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> index 6fbc0441c4b8..2e9676305439 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_rx.c
> @@ -2222,9 +2222,10 @@ static bool mlx5e_hw_gro_skb_has_enough_space(struct sk_buff *skb,
>  	int nr_frags = skb_shinfo(skb)->nr_frags;
>  
>  	if (page_size >= GRO_LEGACY_MAX_SIZE)
> -		return skb->len + data_bcnt <= GRO_LEGACY_MAX_SIZE;
> +		return skb->len + data_bcnt < GRO_LEGACY_MAX_SIZE;
>  	else
> -		return page_size * nr_frags + data_bcnt <= GRO_LEGACY_MAX_SIZE;
> +		return skb_headlen(skb) + page_size * nr_frags + data_bcnt <
> +			GRO_LEGACY_MAX_SIZE;
>  }
>  
>  static void mlx5e_handle_rx_cqe_mpwrq_shampo(struct mlx5e_rq *rq, struct mlx5_cqe64 *cqe)
> 
> base-commit: 51b093a7ba27476e1f639455f005e8d2e75390e4

Reviewed-by: Dragos Tatulea <dtatulea@xxxxxxxxxx>

Thanks,
Dragos