diff mbox

[net] ipv6: distinguish frag queues by device for multicast and link-local packets

Message ID 20151124140711.E07F2A0BE8@unicorn.suse.cz
State Awaiting Upstream
Delegated to: Pablo Neira
Headers show

Commit Message

Michal Kubecek Nov. 24, 2015, 2:07 p.m. UTC
If a fragmented multicast packet is received on an ethernet device which
has an active macvlan on top of it, each fragment is duplicated and
received both on the underlying device and the macvlan. If some
fragments for macvlan are processed before the whole packet for the
underlying device is reassembled, the "overlapping fragments" test in
ip6_frag_queue() discards the whole fragment queue.

To resolve this, add device ifindex to the search key and require it to
match reassembling multicast packets and packets to link-local
addresses.

Note: similar patch has been already submitted by Yoshifuji Hideaki in

  http://patchwork.ozlabs.org/patch/220979/

but got lost and forgotten for some reason.

Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
---
 include/net/ipv6.h                      |  1 +
 net/ipv6/netfilter/nf_conntrack_reasm.c |  5 +++--
 net/ipv6/reassembly.c                   | 10 +++++++---
 3 files changed, 11 insertions(+), 5 deletions(-)

Comments

David Miller Nov. 24, 2015, 9:46 p.m. UTC | #1
From: Michal Kubecek <mkubecek@suse.cz>
Date: Tue, 24 Nov 2015 15:07:11 +0100 (CET)

> If a fragmented multicast packet is received on an ethernet device which
> has an active macvlan on top of it, each fragment is duplicated and
> received both on the underlying device and the macvlan. If some
> fragments for macvlan are processed before the whole packet for the
> underlying device is reassembled, the "overlapping fragments" test in
> ip6_frag_queue() discards the whole fragment queue.
> 
> To resolve this, add device ifindex to the search key and require it to
> match reassembling multicast packets and packets to link-local
> addresses.
> 
> Note: similar patch has been already submitted by Yoshifuji Hideaki in
> 
>   http://patchwork.ozlabs.org/patch/220979/
> 
> but got lost and forgotten for some reason.
> 
> Signed-off-by: Michal Kubecek <mkubecek@suse.cz>

This is definitely the right thing to do and matches how ipv4 keys
fragments.

Applied and queued up for -stable, 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
Hannes Frederic Sowa Nov. 24, 2015, 10:01 p.m. UTC | #2
On Tue, Nov 24, 2015, at 22:46, David Miller wrote:
> From: Michal Kubecek <mkubecek@suse.cz>
> Date: Tue, 24 Nov 2015 15:07:11 +0100 (CET)
> 
> > If a fragmented multicast packet is received on an ethernet device which
> > has an active macvlan on top of it, each fragment is duplicated and
> > received both on the underlying device and the macvlan. If some
> > fragments for macvlan are processed before the whole packet for the
> > underlying device is reassembled, the "overlapping fragments" test in
> > ip6_frag_queue() discards the whole fragment queue.
> > 
> > To resolve this, add device ifindex to the search key and require it to
> > match reassembling multicast packets and packets to link-local
> > addresses.
> > 
> > Note: similar patch has been already submitted by Yoshifuji Hideaki in
> > 
> >   http://patchwork.ozlabs.org/patch/220979/
> > 
> > but got lost and forgotten for some reason.
> > 
> > Signed-off-by: Michal Kubecek <mkubecek@suse.cz>
> 
> This is definitely the right thing to do and matches how ipv4 keys
> fragments.
> 
> Applied and queued up for -stable, thanks!

I reviewed it earlier and agree last time that this patch is necessary.
Unfortunately forgot to ack before. :(

Acked-by: Hannes Frederic Sowa <hannes@stressinduktion.org>

In IPv4 as in IPv6 global addresses we have to expect packets coming
over multiple interfaces, it is only correct for local and multicast
scoped addresses. In IPv4 we don't really key the device index, only in
case of an vrf interface.

Thanks,
Hannes
--
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 mbox

Patch

diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index e1a10b0ac0b0..ea5a13ef85a6 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -490,6 +490,7 @@  struct ip6_create_arg {
 	u32 user;
 	const struct in6_addr *src;
 	const struct in6_addr *dst;
+	int iif;
 	u8 ecn;
 };
 
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
index d5efeb87350e..bab4441ed4e4 100644
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
@@ -190,7 +190,7 @@  static void nf_ct_frag6_expire(unsigned long data)
 /* Creation primitives. */
 static inline struct frag_queue *fq_find(struct net *net, __be32 id,
 					 u32 user, struct in6_addr *src,
-					 struct in6_addr *dst, u8 ecn)
+					 struct in6_addr *dst, int iif, u8 ecn)
 {
 	struct inet_frag_queue *q;
 	struct ip6_create_arg arg;
@@ -200,6 +200,7 @@  static inline struct frag_queue *fq_find(struct net *net, __be32 id,
 	arg.user = user;
 	arg.src = src;
 	arg.dst = dst;
+	arg.iif = iif;
 	arg.ecn = ecn;
 
 	local_bh_disable();
@@ -601,7 +602,7 @@  struct sk_buff *nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 use
 	fhdr = (struct frag_hdr *)skb_transport_header(clone);
 
 	fq = fq_find(net, fhdr->identification, user, &hdr->saddr, &hdr->daddr,
-		     ip6_frag_ecn(hdr));
+		     skb->dev ? skb->dev->ifindex : 0, ip6_frag_ecn(hdr));
 	if (fq == NULL) {
 		pr_debug("Can't find and can't create new queue\n");
 		goto ret_orig;
diff --git a/net/ipv6/reassembly.c b/net/ipv6/reassembly.c
index 44e21a03cfc3..45f5ae51de65 100644
--- a/net/ipv6/reassembly.c
+++ b/net/ipv6/reassembly.c
@@ -108,7 +108,10 @@  bool ip6_frag_match(const struct inet_frag_queue *q, const void *a)
 	return	fq->id == arg->id &&
 		fq->user == arg->user &&
 		ipv6_addr_equal(&fq->saddr, arg->src) &&
-		ipv6_addr_equal(&fq->daddr, arg->dst);
+		ipv6_addr_equal(&fq->daddr, arg->dst) &&
+		(arg->iif == fq->iif ||
+		 !(ipv6_addr_type(arg->dst) & (IPV6_ADDR_MULTICAST |
+					       IPV6_ADDR_LINKLOCAL)));
 }
 EXPORT_SYMBOL(ip6_frag_match);
 
@@ -180,7 +183,7 @@  static void ip6_frag_expire(unsigned long data)
 
 static struct frag_queue *
 fq_find(struct net *net, __be32 id, const struct in6_addr *src,
-	const struct in6_addr *dst, u8 ecn)
+	const struct in6_addr *dst, int iif, u8 ecn)
 {
 	struct inet_frag_queue *q;
 	struct ip6_create_arg arg;
@@ -190,6 +193,7 @@  fq_find(struct net *net, __be32 id, const struct in6_addr *src,
 	arg.user = IP6_DEFRAG_LOCAL_DELIVER;
 	arg.src = src;
 	arg.dst = dst;
+	arg.iif = iif;
 	arg.ecn = ecn;
 
 	hash = inet6_hash_frag(id, src, dst);
@@ -551,7 +555,7 @@  static int ipv6_frag_rcv(struct sk_buff *skb)
 	}
 
 	fq = fq_find(net, fhdr->identification, &hdr->saddr, &hdr->daddr,
-		     ip6_frag_ecn(hdr));
+		     skb->dev ? skb->dev->ifindex : 0, ip6_frag_ecn(hdr));
 	if (fq) {
 		int ret;