| Message ID | 22776.44325.616928.335465@gargle.gargle.HOWL |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
On Thu, Apr 20, 2017 at 08:44:21AM -0400, Dave Johnson wrote: > > When recalculating the outer ICMPv6 checksum for a reverse path NATv6 > such as ICMPV6_TIME_EXCEED nf_nat_icmpv6_reply_translation() was > accessing data beyond the headlen of the skb for non-linear skb. This > resulted in incorrect ICMPv6 checksum as garbage data was used. > > Signed-off-by: Dave Johnson <dave-kernel@centerclick.org> > --- > diff -rup linux-4.9.23.orig/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c linux-4.9.23/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c > --- linux-4.9.23.orig/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c 2017-04-18 01:12:30.000000000 -0400 > +++ linux-4.9.23/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c 2017-04-20 08:13:41.070493666 -0400 > @@ -229,7 +229,12 @@ int nf_nat_icmpv6_reply_translation(stru > return 0; > > if (skb->ip_summed != CHECKSUM_PARTIAL) { > - struct ipv6hdr *ipv6h = ipv6_hdr(skb); > + struct ipv6hdr *ipv6h; > + > + if (!skb_make_writable(skb, skb->len)) can we just make sure what we need is linear? I mean, just the ipv6 header that is what we need, instead of the entire skbuff. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Pablo Neira Ayuso writes: > On Thu, Apr 20, 2017 at 08:44:21AM -0400, Dave Johnson wrote: > > > > When recalculating the outer ICMPv6 checksum for a reverse path NATv6 > > such as ICMPV6_TIME_EXCEED nf_nat_icmpv6_reply_translation() was > > accessing data beyond the headlen of the skb for non-linear skb. This > > resulted in incorrect ICMPv6 checksum as garbage data was used. > > > > Signed-off-by: Dave Johnson <dave-kernel@centerclick.org> > > --- > > diff -rup linux-4.9.23.orig/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c linux-4.9.23/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c > > --- linux-4.9.23.orig/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c 2017-04-18 01:12:30.000000000 -0400 > > +++ linux-4.9.23/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c 2017-04-20 08:13:41.070493666 -0400 > > @@ -229,7 +229,12 @@ int nf_nat_icmpv6_reply_translation(stru > > return 0; > > > > if (skb->ip_summed != CHECKSUM_PARTIAL) { > > - struct ipv6hdr *ipv6h = ipv6_hdr(skb); > > + struct ipv6hdr *ipv6h; > > + > > + if (!skb_make_writable(skb, skb->len)) > > can we just make sure what we need is linear? I mean, just the ipv6 > header that is what we need, instead of the entire skbuff. the checksum below those lines is across the entire skb as unknown updates were done in the l4 manip call just prior to this. nf_nat_icmp_reply_translation() for ipv4 uses skb_checksum() to walk non-linear skbs. gave that a try and it works, will send an updated patch in a bit. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Apr 20, 2017 at 08:44:21AM -0400, Dave Johnson wrote: > > When recalculating the outer ICMPv6 checksum for a reverse path NATv6 > such as ICMPV6_TIME_EXCEED nf_nat_icmpv6_reply_translation() was > accessing data beyond the headlen of the skb for non-linear skb. This > resulted in incorrect ICMPv6 checksum as garbage data was used. Applied to nf, thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff -rup linux-4.9.23.orig/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c linux-4.9.23/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c --- linux-4.9.23.orig/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c 2017-04-18 01:12:30.000000000 -0400 +++ linux-4.9.23/net/ipv6/netfilter/nf_nat_l3proto_ipv6.c 2017-04-20 08:13:41.070493666 -0400 @@ -229,7 +229,12 @@ int nf_nat_icmpv6_reply_translation(stru return 0; if (skb->ip_summed != CHECKSUM_PARTIAL) { - struct ipv6hdr *ipv6h = ipv6_hdr(skb); + struct ipv6hdr *ipv6h; + + if (!skb_make_writable(skb, skb->len)) + return 0; + + ipv6h = ipv6_hdr(skb); inside = (void *)skb->data + hdrlen; inside->icmp6.icmp6_cksum = 0; inside->icmp6.icmp6_cksum =
When recalculating the outer ICMPv6 checksum for a reverse path NATv6 such as ICMPV6_TIME_EXCEED nf_nat_icmpv6_reply_translation() was accessing data beyond the headlen of the skb for non-linear skb. This resulted in incorrect ICMPv6 checksum as garbage data was used. Signed-off-by: Dave Johnson <dave-kernel@centerclick.org> --- -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html