netdev - Re: [PATCH net 2/2] net: ionic: map SKB after pseudo-header checksum prep

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <594de4e3-90f5-489a-8cc8-ddc04149d1ac@amd.com>
Date: Fri, 31 Oct 2025 13:11:11 -0700
From: Brett Creeley <bcreeley@....com>
To: mheib@...hat.com, netdev@...r.kernel.org
Cc: brett.creeley@....com, andrew+netdev@...n.ch, davem@...emloft.net,
 kuba@...nel.org
Subject: Re: [PATCH net 2/2] net: ionic: map SKB after pseudo-header checksum
 prep



On 10/31/2025 8:52 AM, mheib@...hat.com wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
> 
> 
> From: Mohammad Heib <mheib@...hat.com>
> 
> The TSO path called ionic_tx_map_skb() before preparing the TCP pseudo
> checksum (ionic_tx_tcp_[inner_]pseudo_csum()), which may perform
> skb_cow_head() and might modifies bytes in the linear header area.
> 
> Mapping first and then mutating the header risks:
>    - Using a stale DMA address if skb_cow_head() relocates the head, and/or
>    - Device reading stale header bytes on weakly-ordered systems
>      (CPU writes after mapping are not guaranteed visible without an
>      explicit dma_sync_single_for_device()).
> 
> Reorder the TX path to perform all header mutations (including
> skb_cow_head()) *before* DMA mapping. Mapping is now done only after the
> skb layout and header contents are final. This removes the need for any
> post-mapping dma_sync and prevents on-wire corruption observed under
> VLAN+TSO load after repeated runs.
> 
> This change is purely an ordering fix; no functional behavior change
> otherwise.
> 
> Fixes: 0f3154e6bcb3 ("ionic: Add Tx and Rx handling")
> Signed-off-by: Mohammad Heib <mheib@...hat.com>
> ---
>   .../net/ethernet/pensando/ionic/ionic_txrx.c  | 30 ++++++++-----------
>   1 file changed, 13 insertions(+), 17 deletions(-)

Thanks for the fix! LGTM.

Reviewed-by: Brett Creeley <brett.creeley@....com>

> 
> diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> index 2e571d0a0d8a..301ebee2fdc5 100644
> --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c
> @@ -1448,19 +1448,6 @@ static int ionic_tx_tso(struct net_device *netdev, struct ionic_queue *q,
>          bool encap;
>          int err;
> 
> -       desc_info = &q->tx_info[q->head_idx];
> -
> -       if (unlikely(ionic_tx_map_skb(q, skb, desc_info)))
> -               return -EIO;
> -
> -       len = skb->len;
> -       mss = skb_shinfo(skb)->gso_size;
> -       outer_csum = (skb_shinfo(skb)->gso_type & (SKB_GSO_GRE |
> -                                                  SKB_GSO_GRE_CSUM |
> -                                                  SKB_GSO_IPXIP4 |
> -                                                  SKB_GSO_IPXIP6 |
> -                                                  SKB_GSO_UDP_TUNNEL |
> -                                                  SKB_GSO_UDP_TUNNEL_CSUM));
>          has_vlan = !!skb_vlan_tag_present(skb);
>          vlan_tci = skb_vlan_tag_get(skb);
>          encap = skb->encapsulation;
> @@ -1474,12 +1461,21 @@ static int ionic_tx_tso(struct net_device *netdev, struct ionic_queue *q,
>                  err = ionic_tx_tcp_inner_pseudo_csum(skb);
>          else
>                  err = ionic_tx_tcp_pseudo_csum(skb);
> -       if (unlikely(err)) {
> -               /* clean up mapping from ionic_tx_map_skb */
> -               ionic_tx_desc_unmap_bufs(q, desc_info);
> +       if (unlikely(err))
>                  return err;
> -       }
> 
> +       desc_info = &q->tx_info[q->head_idx];
> +       if (unlikely(ionic_tx_map_skb(q, skb, desc_info)))
> +               return -EIO;
> +
> +       len = skb->len;
> +       mss = skb_shinfo(skb)->gso_size;
> +       outer_csum = (skb_shinfo(skb)->gso_type & (SKB_GSO_GRE |
> +                                                  SKB_GSO_GRE_CSUM |
> +                                                  SKB_GSO_IPXIP4 |
> +                                                  SKB_GSO_IPXIP6 |
> +                                                  SKB_GSO_UDP_TUNNEL |
> +                                                  SKB_GSO_UDP_TUNNEL_CSUM));
>          if (encap)
>                  hdrlen = skb_inner_tcp_all_headers(skb);
>          else
> --
> 2.50.1
>