netdev - Re: [External] Re: [PATCH] udp:nat:vxlan tx after nat should recsum if vxlan tx offload on

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] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 4 Apr 2023 11:26:07 +0100
From:   Edward Cree <ecree.xilinx@...il.com>
To:     Fei Cheng <chenwei.0515@...edance.com>,
        Willem de Bruijn <willemdebruijn.kernel@...il.com>
Cc:     dsahern@...nel.org, davem@...emloft.net,
        netfilter-devel@...r.kernel.org,
        Network Development <netdev@...r.kernel.org>, ecree@....com
Subject: Re: [External] Re: [PATCH] udp:nat:vxlan tx after nat should recsum
 if vxlan tx offload on

On 04/04/2023 02:48, Fei Cheng wrote:
> Thank you for remind plain text.
> Use csum_start to seperate these two cases, maybe a good idea.
> 1.Disable tx csum
> skb->ip_summed ==  CHECKSUM_PARTIAL && skb_transport_header == udp
> 2.Enable tx csum
> skb->ip_summed ==  CHECKSUM_PARTIAL && skb_transport_header != udp
> 
> Correct?

What do you mean by "skb_transport_header == udp"?  That it is a UDP
 header?  Or that it is at the offset of the UDP header?  The inner
 L4 packet could be UDP as well.
And why are you even looking at skb_transport_header when it's
 csum_start that determines what the hardware will do?
AFAICT nothing in nf_nat_proto.c looks at skb_transport_header anyway;
 indeed in nf_nat_icmp_reply_translation() we can call into this code
 with hdroff ending up pointing way deeper in the packet.

In any case, after digging deeper into the netfilter code, it looks to
 me like there's no issue in the first place: netfilter doesn't
 'recompute' the UDP checksum, it just accumulates delta into it to
 account for the changes it made to the packet, on the assumption that
 the existing checksum is correct.
Which AIUI will do the right thing whether the checksum is
* a correct checksum for an unencapsulated packet
* a pseudohdr sum for a CHECKSUM_PARTIAL (unencapsulated) packet
* a correct (LCO) checksum for an encapsulated packet, whose inner L4
  checksum may or may not be CHECKSUM_PARTIAL offloaded.

Do you have a test case that breaks with the current code?

-ed