Improve zero checksum handling

As per the suggestion by @ayourtch:

Well setting zero_csum_pass to 1 is not much of a fix, is it - it rather masks the problem in the code. Also it is a change of defaults, which i would be a bit hesitant to do…

Here’s an idea, what if we rewrite that bit as follows:

if (!udp->check) {
if (zero_csum_pass) {
// do nothing
break;
}
// recalculate the IPv6 checksum before adjusting
ip6_update_csum(…)
}
// here we will have nonzero checksum, unmagic + remagic

so, if it’s UDP and the checksum is zero and we don’t pass the zero checksum, recalculate it ?

It will still be broken if we don’t have full payload since we can’t do the full calculation in this case, but should take care of other cases ?

what do you think ?

Author: angus19

nat46/modules/nat46-core.c

@@ -1007,6 +1007,7 @@ int xlate_payload6_to4(nat46_instance_t *nat46, void *pv6, void *ptrans_hdr, int
         /* zero checksum and the config to pass it is set - do nothing with it */
         break;
       }
+      /* pretend checksum is correct not null */
       sum1 = csum_ipv6_unmagic(nat46, &ip6h->saddr, &ip6h->daddr, infrag_payload_len, NEXTHDR_UDP, udp->check);
       sum2 = csum_tcpudp_remagic(v4saddr, v4daddr, infrag_payload_len, NEXTHDR_UDP, sum1); /* add pseudoheader */
       if(ul_sum) {
@@ -1671,9 +1672,12 @@ int nat46_ipv6_input(struct sk_buff *old_skb) {
       case NEXTHDR_UDP: {
         struct udphdr *udp = add_offset(ip6h, v6packet_l3size);
         u16 sum1, sum2;
-        if ((udp->check == 0) && zero_csum_pass) {
-          /* zero checksum and the config to pass it is set - do nothing with it */
-          break;
+        if (udp->check == 0) {
+          if (zero_csum_pass) {
+            /* zero checksum and the config to pass it is set - do nothing with it */
+            break;
+          }
+          ip6_update_csum(old_skb, ip6h, ip6h->nexthdr == NEXTHDR_FRAGMENT); /* recalculate checksum before adjusting */
         }
         sum1 = csum_ipv6_unmagic(nat46, &ip6h->saddr, &ip6h->daddr, l3_infrag_payload_len, NEXTHDR_UDP, udp->check);
         sum2 = csum_tcpudp_remagic(v4saddr, v4daddr, l3_infrag_payload_len, NEXTHDR_UDP, sum1);
@@ -1767,8 +1771,17 @@ void ip6_update_csum(struct sk_buff * skb, struct ipv6hdr * ip6hdr, int do_atomi
       unsigned udplen = ntohs(ip6hdr->payload_len) - (do_atomic_frag?8:0); /* UDP hdr + payload */
 
       if ((udp->check == 0) && zero_csum_pass) {
-         /* zero checksum and the config to pass it is set - do nothing with it */
-         break;
+        /* zero checksum and the config to pass it is set - do nothing with it */
+        break;
+      }
+
+      if (do_atomic_frag) {
+        if ((udp->check == 0) && !zero_csum_pass) {
+          /*
+           * Checksum will still be broken if we don't have full payload
+           * since we can't do the full calculation in this case.
+           */
+        }
       }
 
       oldsum = udp->check;

nat46/modules/nat46-core.h

@@ -82,4 +82,6 @@ nat46_instance_t *get_nat46_instance(struct sk_buff *sk);
 nat46_instance_t *alloc_nat46_instance(int npairs, nat46_instance_t *old, int from_ipair, int to_ipair, int remove_ipair);
 void release_nat46_instance(nat46_instance_t *nat46);
 
+void ip6_update_csum(struct sk_buff * skb, struct ipv6hdr * ip6hdr, int do_atomic_frag);
+
 #endif