|
 |
b2248a2 |
From 9b57da0630c9fd36ed7a20fc0f98dc82cc0777fa Mon Sep 17 00:00:00 2001
|
|
|
b2248a2 |
From: Florian Westphal <fw@strlen.de>
|
|
|
b2248a2 |
Date: Tue, 29 Nov 2016 02:17:34 +0100
|
|
|
b2248a2 |
Subject: [PATCH] netfilter: ipv6: nf_defrag: drop mangled skb on ream error
|
|
|
b2248a2 |
|
|
|
b2248a2 |
Dmitry Vyukov reported GPF in network stack that Andrey traced down to
|
|
|
b2248a2 |
negative nh offset in nf_ct_frag6_queue().
|
|
|
b2248a2 |
|
|
|
b2248a2 |
Problem is that all network headers before fragment header are pulled.
|
|
|
b2248a2 |
Normal ipv6 reassembly will drop the skb when errors occur further down
|
|
|
b2248a2 |
the line.
|
|
|
b2248a2 |
|
|
|
b2248a2 |
netfilter doesn't do this, and instead passed the original fragment
|
|
|
b2248a2 |
along. That was also fine back when netfilter ipv6 defrag worked with
|
|
|
b2248a2 |
cloned fragments, as the original, pristine fragment was passed on.
|
|
|
b2248a2 |
|
|
|
b2248a2 |
So we either have to undo the pull op, or discard such fragments.
|
|
|
b2248a2 |
Since they're malformed after all (e.g. overlapping fragment) it seems
|
|
|
b2248a2 |
preferrable to just drop them.
|
|
|
b2248a2 |
|
|
|
b2248a2 |
Same for temporary errors -- it doesn't make sense to accept (and
|
|
|
b2248a2 |
perhaps forward!) only some fragments of same datagram.
|
|
|
b2248a2 |
|
|
|
b2248a2 |
Fixes: 029f7f3b8701cc7ac ("netfilter: ipv6: nf_defrag: avoid/free clone operations")
|
|
|
b2248a2 |
Reported-by: Dmitry Vyukov <dvyukov@google.com>
|
|
|
b2248a2 |
Debugged-by: Andrey Konovalov <andreyknvl@google.com>
|
|
|
b2248a2 |
Diagnosed-by: Eric Dumazet <Eric Dumazet <edumazet@google.com>
|
|
|
b2248a2 |
Signed-off-by: Florian Westphal <fw@strlen.de>
|
|
|
b2248a2 |
Acked-by: Eric Dumazet <edumazet@google.com>
|
|
|
b2248a2 |
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
|
|
|
b2248a2 |
---
|
|
|
b2248a2 |
net/ipv6/netfilter/nf_conntrack_reasm.c | 4 ++--
|
|
|
b2248a2 |
net/ipv6/netfilter/nf_defrag_ipv6_hooks.c | 2 +-
|
|
|
b2248a2 |
2 files changed, 3 insertions(+), 3 deletions(-)
|
|
|
b2248a2 |
|
|
|
b2248a2 |
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c
|
|
|
b2248a2 |
index e4347ae..9948b5c 100644
|
|
|
b2248a2 |
--- a/net/ipv6/netfilter/nf_conntrack_reasm.c
|
|
|
b2248a2 |
+++ b/net/ipv6/netfilter/nf_conntrack_reasm.c
|
|
|
b2248a2 |
@@ -576,11 +576,11 @@ int nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 user)
|
|
|
b2248a2 |
/* Jumbo payload inhibits frag. header */
|
|
|
b2248a2 |
if (ipv6_hdr(skb)->payload_len == 0) {
|
|
|
b2248a2 |
pr_debug("payload len = 0\n");
|
|
|
b2248a2 |
- return -EINVAL;
|
|
|
b2248a2 |
+ return 0;
|
|
|
b2248a2 |
}
|
|
|
b2248a2 |
|
|
|
b2248a2 |
if (find_prev_fhdr(skb, &prevhdr, &nhoff, &fhoff) < 0)
|
|
|
b2248a2 |
- return -EINVAL;
|
|
|
b2248a2 |
+ return 0;
|
|
|
b2248a2 |
|
|
|
b2248a2 |
if (!pskb_may_pull(skb, fhoff + sizeof(*fhdr)))
|
|
|
b2248a2 |
return -ENOMEM;
|
|
|
b2248a2 |
diff --git a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
|
|
|
b2248a2 |
index f7aab5a..f06b047 100644
|
|
|
b2248a2 |
--- a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
|
|
|
b2248a2 |
+++ b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c
|
|
|
b2248a2 |
@@ -69,7 +69,7 @@ static unsigned int ipv6_defrag(void *priv,
|
|
|
b2248a2 |
if (err == -EINPROGRESS)
|
|
|
b2248a2 |
return NF_STOLEN;
|
|
|
b2248a2 |
|
|
|
b2248a2 |
- return NF_ACCEPT;
|
|
|
b2248a2 |
+ return err == 0 ? NF_ACCEPT : NF_DROP;
|
|
|
b2248a2 |
}
|
|
|
b2248a2 |
|
|
|
b2248a2 |
static struct nf_hook_ops ipv6_defrag_ops[] = {
|
|
|
b2248a2 |
--
|
|
|
b2248a2 |
2.9.3
|
|
|
b2248a2 |
|