Commit 7b4fdf77 authored by Florian Westphal's avatar Florian Westphal Committed by Pablo Neira Ayuso

netfilter: don't track fragmented packets

Andrey reports syzkaller splat caused by

NF_CT_ASSERT(!ip_is_fragment(ip_hdr(skb)));

in ipv4 nat.  But this assertion (and the comment) are wrong, this function
does see fragments when IP_NODEFRAG setsockopt is used.

As conntrack doesn't track packets without complete l4 header, only the
first fragment is tracked.

Because applying nat to first packet but not the rest makes no sense this
also turns off tracking of all fragments.
Reported-by: default avatarAndrey Konovalov <andreyknvl@google.com>
Signed-off-by: default avatarFlorian Westphal <fw@strlen.de>
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent 8d70eeb8
...@@ -165,6 +165,10 @@ static unsigned int ipv4_conntrack_local(void *priv, ...@@ -165,6 +165,10 @@ static unsigned int ipv4_conntrack_local(void *priv,
if (skb->len < sizeof(struct iphdr) || if (skb->len < sizeof(struct iphdr) ||
ip_hdrlen(skb) < sizeof(struct iphdr)) ip_hdrlen(skb) < sizeof(struct iphdr))
return NF_ACCEPT; return NF_ACCEPT;
if (ip_is_fragment(ip_hdr(skb))) /* IP_NODEFRAG setsockopt set */
return NF_ACCEPT;
return nf_conntrack_in(state->net, PF_INET, state->hook, skb); return nf_conntrack_in(state->net, PF_INET, state->hook, skb);
} }
......
...@@ -255,11 +255,6 @@ nf_nat_ipv4_fn(void *priv, struct sk_buff *skb, ...@@ -255,11 +255,6 @@ nf_nat_ipv4_fn(void *priv, struct sk_buff *skb,
/* maniptype == SRC for postrouting. */ /* maniptype == SRC for postrouting. */
enum nf_nat_manip_type maniptype = HOOK2MANIP(state->hook); enum nf_nat_manip_type maniptype = HOOK2MANIP(state->hook);
/* We never see fragments: conntrack defrags on pre-routing
* and local-out, and nf_nat_out protects post-routing.
*/
NF_CT_ASSERT(!ip_is_fragment(ip_hdr(skb)));
ct = nf_ct_get(skb, &ctinfo); ct = nf_ct_get(skb, &ctinfo);
/* Can't track? It's not due to stress, or conntrack would /* Can't track? It's not due to stress, or conntrack would
* have dropped it. Hence it's the user's responsibilty to * have dropped it. Hence it's the user's responsibilty to
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment