From 013e8167899d389075160412a8c0c5e0581e1f13 Mon Sep 17 00:00:00 2001 From: David Lebrun Date: Thu, 2 Feb 2017 11:29:38 +0100 Subject: ipv6: sr: remove cleanup flag and fix HMAC computation In the latest version of the IPv6 Segment Routing IETF draft [1] the cleanup flag is removed and the flags field length is shrunk from 16 bits to 8 bits. As a consequence, the input of the HMAC computation is modified in a non-backward compatible way by covering the whole octet of flags instead of only the cleanup bit. As such, if an implementation compatible with the latest draft computes the HMAC of an SRH who has other flags set to 1, then the HMAC result would differ from the current implementation. This patch carries those modifications to prevent conflict with other implementations of IPv6 SR. [1] https://tools.ietf.org/html/draft-ietf-6man-segment-routing-header-05 Signed-off-by: David Lebrun Signed-off-by: David S. Miller --- net/ipv6/exthdrs.c | 31 +++---------------------------- net/ipv6/seg6_hmac.c | 8 ++++---- 2 files changed, 7 insertions(+), 32 deletions(-) (limited to 'net/ipv6') diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c index e419850..275cac6 100644 --- a/net/ipv6/exthdrs.c +++ b/net/ipv6/exthdrs.c @@ -327,7 +327,6 @@ static int ipv6_srh_rcv(struct sk_buff *skb) struct ipv6_sr_hdr *hdr; struct inet6_dev *idev; struct in6_addr *addr; - bool cleanup = false; int accept_seg6; hdr = (struct ipv6_sr_hdr *)skb_transport_header(skb); @@ -351,11 +350,7 @@ static int ipv6_srh_rcv(struct sk_buff *skb) #endif looped_back: - if (hdr->segments_left > 0) { - if (hdr->nexthdr != NEXTHDR_IPV6 && hdr->segments_left == 1 && - sr_has_cleanup(hdr)) - cleanup = true; - } else { + if (hdr->segments_left == 0) { if (hdr->nexthdr == NEXTHDR_IPV6) { int offset = (hdr->hdrlen + 1) << 3; @@ -418,21 +413,6 @@ looped_back: ipv6_hdr(skb)->daddr = *addr; - if (cleanup) { - int srhlen = (hdr->hdrlen + 1) << 3; - int nh = hdr->nexthdr; - - skb_pull_rcsum(skb, sizeof(struct ipv6hdr) + srhlen); - memmove(skb_network_header(skb) + srhlen, - skb_network_header(skb), - (unsigned char *)hdr - skb_network_header(skb)); - skb->network_header += srhlen; - ipv6_hdr(skb)->nexthdr = nh; - ipv6_hdr(skb)->payload_len = htons(skb->len - - sizeof(struct ipv6hdr)); - skb_push_rcsum(skb, sizeof(struct ipv6hdr)); - } - skb_dst_drop(skb); ip6_route_input(skb); @@ -453,13 +433,8 @@ looped_back: } ipv6_hdr(skb)->hop_limit--; - /* be sure that srh is still present before reinjecting */ - if (!cleanup) { - skb_pull(skb, sizeof(struct ipv6hdr)); - goto looped_back; - } - skb_set_transport_header(skb, sizeof(struct ipv6hdr)); - IP6CB(skb)->nhoff = offsetof(struct ipv6hdr, nexthdr); + skb_pull(skb, sizeof(struct ipv6hdr)); + goto looped_back; } dst_input(skb); diff --git a/net/ipv6/seg6_hmac.c b/net/ipv6/seg6_hmac.c index 03a0648..6ef3dfb 100644 --- a/net/ipv6/seg6_hmac.c +++ b/net/ipv6/seg6_hmac.c @@ -174,7 +174,7 @@ int seg6_hmac_compute(struct seg6_hmac_info *hinfo, struct ipv6_sr_hdr *hdr, * hash function (RadioGatun) with up to 1216 bits */ - /* saddr(16) + first_seg(1) + cleanup(1) + keyid(4) + seglist(16n) */ + /* saddr(16) + first_seg(1) + flags(1) + keyid(4) + seglist(16n) */ plen = 16 + 1 + 1 + 4 + (hdr->first_segment + 1) * 16; /* this limit allows for 14 segments */ @@ -186,7 +186,7 @@ int seg6_hmac_compute(struct seg6_hmac_info *hinfo, struct ipv6_sr_hdr *hdr, * * 1. Source IPv6 address (128 bits) * 2. first_segment value (8 bits) - * 3. cleanup flag (8 bits: highest bit is cleanup value, others are 0) + * 3. Flags (8 bits) * 4. HMAC Key ID (32 bits) * 5. All segments in the segments list (n * 128 bits) */ @@ -202,8 +202,8 @@ int seg6_hmac_compute(struct seg6_hmac_info *hinfo, struct ipv6_sr_hdr *hdr, /* first_segment value */ *off++ = hdr->first_segment; - /* cleanup flag */ - *off++ = !!(sr_has_cleanup(hdr)) << 7; + /* flags */ + *off++ = hdr->flags; /* HMAC Key ID */ memcpy(off, &hmackeyid, 4); -- cgit v1.1 From 7892032cfe67f4bde6fc2ee967e45a8fbaf33756 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Sat, 4 Feb 2017 23:18:55 -0800 Subject: ip6_gre: fix ip6gre_err() invalid reads Andrey Konovalov reported out of bound accesses in ip6gre_err() If GRE flags contains GRE_KEY, the following expression *(((__be32 *)p) + (grehlen / 4) - 1) accesses data ~40 bytes after the expected point, since grehlen includes the size of IPv6 headers. Let's use a "struct gre_base_hdr *greh" pointer to make this code more readable. p[1] becomes greh->protocol. grhlen is the GRE header length. Fixes: c12b395a4664 ("gre: Support GRE over IPv6") Signed-off-by: Eric Dumazet Reported-by: Andrey Konovalov Signed-off-by: David S. Miller --- net/ipv6/ip6_gre.c | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) (limited to 'net/ipv6') diff --git a/net/ipv6/ip6_gre.c b/net/ipv6/ip6_gre.c index 5586318..630b73b 100644 --- a/net/ipv6/ip6_gre.c +++ b/net/ipv6/ip6_gre.c @@ -367,35 +367,37 @@ static void ip6gre_tunnel_uninit(struct net_device *dev) static void ip6gre_err(struct sk_buff *skb, struct inet6_skb_parm *opt, - u8 type, u8 code, int offset, __be32 info) + u8 type, u8 code, int offset, __be32 info) { - const struct ipv6hdr *ipv6h = (const struct ipv6hdr *)skb->data; - __be16 *p = (__be16 *)(skb->data + offset); - int grehlen = offset + 4; + const struct gre_base_hdr *greh; + const struct ipv6hdr *ipv6h; + int grehlen = sizeof(*greh); struct ip6_tnl *t; + int key_off = 0; __be16 flags; + __be32 key; - flags = p[0]; - if (flags&(GRE_CSUM|GRE_KEY|GRE_SEQ|GRE_ROUTING|GRE_VERSION)) { - if (flags&(GRE_VERSION|GRE_ROUTING)) - return; - if (flags&GRE_KEY) { - grehlen += 4; - if (flags&GRE_CSUM) - grehlen += 4; - } + if (!pskb_may_pull(skb, offset + grehlen)) + return; + greh = (const struct gre_base_hdr *)(skb->data + offset); + flags = greh->flags; + if (flags & (GRE_VERSION | GRE_ROUTING)) + return; + if (flags & GRE_CSUM) + grehlen += 4; + if (flags & GRE_KEY) { + key_off = grehlen + offset; + grehlen += 4; } - /* If only 8 bytes returned, keyed message will be dropped here */ - if (!pskb_may_pull(skb, grehlen)) + if (!pskb_may_pull(skb, offset + grehlen)) return; ipv6h = (const struct ipv6hdr *)skb->data; - p = (__be16 *)(skb->data + offset); + greh = (const struct gre_base_hdr *)(skb->data + offset); + key = key_off ? *(__be32 *)(skb->data + key_off) : 0; t = ip6gre_tunnel_lookup(skb->dev, &ipv6h->daddr, &ipv6h->saddr, - flags & GRE_KEY ? - *(((__be32 *)p) + (grehlen / 4) - 1) : 0, - p[1]); + key, greh->protocol); if (!t) return; -- cgit v1.1 From ebf6c9cb23d7e56eec8575a88071dec97ad5c6e2 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Sun, 5 Feb 2017 20:23:22 -0800 Subject: ipv6: tcp: add a missing tcp_v6_restore_cb() Dmitry reported use-after-free in ip6_datagram_recv_specific_ctl() A similar bug was fixed in commit 8ce48623f0cf ("ipv6: tcp: restore IP6CB for pktoptions skbs"), but I missed another spot. tcp_v6_syn_recv_sock() can indeed set np->pktoptions from ireq->pktopts Fixes: 971f10eca186 ("tcp: better TCP_SKB_CB layout to reduce cache line misses") Signed-off-by: Eric Dumazet Reported-by: Dmitry Vyukov Signed-off-by: David S. Miller --- net/ipv6/tcp_ipv6.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) (limited to 'net/ipv6') diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c index cb89296..eaad72c 100644 --- a/net/ipv6/tcp_ipv6.c +++ b/net/ipv6/tcp_ipv6.c @@ -991,6 +991,16 @@ drop: return 0; /* don't send reset */ } +static void tcp_v6_restore_cb(struct sk_buff *skb) +{ + /* We need to move header back to the beginning if xfrm6_policy_check() + * and tcp_v6_fill_cb() are going to be called again. + * ip6_datagram_recv_specific_ctl() also expects IP6CB to be there. + */ + memmove(IP6CB(skb), &TCP_SKB_CB(skb)->header.h6, + sizeof(struct inet6_skb_parm)); +} + static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff *skb, struct request_sock *req, struct dst_entry *dst, @@ -1182,8 +1192,10 @@ static struct sock *tcp_v6_syn_recv_sock(const struct sock *sk, struct sk_buff * sk_gfp_mask(sk, GFP_ATOMIC)); consume_skb(ireq->pktopts); ireq->pktopts = NULL; - if (newnp->pktoptions) + if (newnp->pktoptions) { + tcp_v6_restore_cb(newnp->pktoptions); skb_set_owner_r(newnp->pktoptions, newsk); + } } } @@ -1198,16 +1210,6 @@ out: return NULL; } -static void tcp_v6_restore_cb(struct sk_buff *skb) -{ - /* We need to move header back to the beginning if xfrm6_policy_check() - * and tcp_v6_fill_cb() are going to be called again. - * ip6_datagram_recv_specific_ctl() also expects IP6CB to be there. - */ - memmove(IP6CB(skb), &TCP_SKB_CB(skb)->header.h6, - sizeof(struct inet6_skb_parm)); -} - /* The socket must have it's spinlock held when we get * here, unless it is a TCP_LISTEN socket. * -- cgit v1.1 From a088d1d73a4bcfd7bc482f8d08375b9b665dc3e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20L=C3=BCssing?= Date: Fri, 3 Feb 2017 08:11:03 +0100 Subject: ipv6: Fix IPv6 packet loss in scenarios involving roaming + snooping switches MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When for instance a mobile Linux device roams from one access point to another with both APs sharing the same broadcast domain and a multicast snooping switch in between: 1) (c) <~~~> (AP1) <--[SSW]--> (AP2) 2) (AP1) <--[SSW]--> (AP2) <~~~> (c) Then currently IPv6 multicast packets will get lost for (c) until an MLD Querier sends its next query message. The packet loss occurs because upon roaming the Linux host so far stayed silent regarding MLD and the snooping switch will therefore be unaware of the multicast topology change for a while. This patch fixes this by always resending MLD reports when an interface change happens, for instance from NO-CARRIER to CARRIER state. Signed-off-by: Linus Lüssing Signed-off-by: David S. Miller --- net/ipv6/addrconf.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'net/ipv6') diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index f60e88e..81f7b4e 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -3386,9 +3386,15 @@ static int addrconf_notify(struct notifier_block *this, unsigned long event, } if (idev) { - if (idev->if_flags & IF_READY) - /* device is already configured. */ + if (idev->if_flags & IF_READY) { + /* device is already configured - + * but resend MLD reports, we might + * have roamed and need to update + * multicast snooping switches + */ + ipv6_mc_up(idev); break; + } idev->if_flags |= IF_READY; } -- cgit v1.1 From 69629464e0b587f3711739b3aa2bcdaf2e075276 Mon Sep 17 00:00:00 2001 From: Eric Dumazet Date: Sun, 5 Feb 2017 09:25:24 -0800 Subject: udp: properly cope with csum errors Dmitry reported that UDP sockets being destroyed would trigger the WARN_ON(atomic_read(&sk->sk_rmem_alloc)); in inet_sock_destruct() It turns out we do not properly destroy skb(s) that have wrong UDP checksum. Thanks again to syzkaller team. Fixes : 7c13f97ffde6 ("udp: do fwd memory scheduling on dequeue") Reported-by: Dmitry Vyukov Signed-off-by: Eric Dumazet Cc: Paolo Abeni Cc: Hannes Frederic Sowa Acked-by: Paolo Abeni Signed-off-by: David S. Miller --- net/ipv6/udp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/ipv6') diff --git a/net/ipv6/udp.c b/net/ipv6/udp.c index 4d5c4ee..8990856 100644 --- a/net/ipv6/udp.c +++ b/net/ipv6/udp.c @@ -441,7 +441,7 @@ try_again: return err; csum_copy_err: - if (!__sk_queue_drop_skb(sk, skb, flags)) { + if (!__sk_queue_drop_skb(sk, skb, flags, udp_skb_destructor)) { if (is_udp4) { UDP_INC_STATS(sock_net(sk), UDP_MIB_CSUMERRORS, is_udplite); -- cgit v1.1