Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755522AbcK1Vhy (ORCPT ); Mon, 28 Nov 2016 16:37:54 -0500 Received: from Chamillionaire.breakpoint.cc ([146.0.238.67]:38352 "EHLO Chamillionaire.breakpoint.cc" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753423AbcK1Vhq (ORCPT ); Mon, 28 Nov 2016 16:37:46 -0500 Date: Mon, 28 Nov 2016 22:34:44 +0100 From: Florian Westphal To: Eric Dumazet Cc: Dmitry Vyukov , Florian Westphal , syzkaller , Hannes Frederic Sowa , David Miller , Tom Herbert , Alexander Duyck , Jiri Benc , Sabrina Dubroca , netdev , LKML Subject: Re: net: GPF in eth_header Message-ID: <20161128213444.GA9858@breakpoint.cc> References: <1480359019.18162.79.camel@edumazet-glaptop3.roam.corp.google.com> <1480362459.18162.83.camel@edumazet-glaptop3.roam.corp.google.com> <1480367152.18162.86.camel@edumazet-glaptop3.roam.corp.google.com> <1480367886.18162.88.camel@edumazet-glaptop3.roam.corp.google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1480367886.18162.88.camel@edumazet-glaptop3.roam.corp.google.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1420 Lines: 37 Eric Dumazet wrote: > > Might be a bug added in commit daaa7d647f81f3 > > ("netfilter: ipv6: avoid nf_iterate recursion") > > > > Florian, what do you think of dropping a packet that presumably was > > mangled badly by nf_ct_frag6_queue() ? ipv4 definitely frees malformed packets. In general, I think netfilter should avoid 'silent' drops if possible and let skb continue, but of course such skbs should not be made worse as what we ate to begin with... > > (Like about 48 byte pulled :(, and/or skb->csum changed ) I think this warrants a review of ipv6 reassembly too, bug reported here is because ipv6 nf defrag is also done on output. > diff --git a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c > index f7aab5ab93a5..508739a3ca2a 100644 > --- a/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c > +++ b/net/ipv6/netfilter/nf_defrag_ipv6_hooks.c > @@ -65,9 +65,9 @@ static unsigned int ipv6_defrag(void *priv, > > err = nf_ct_frag6_gather(state->net, skb, > nf_ct6_defrag_user(state->hook, skb)); > - /* queued */ > - if (err == -EINPROGRESS) > - return NF_STOLEN; > + /* queued or mangled ... */ > + if (err) > + return (err == -EINPROGRESS) ? NF_STOLEN : NF_DROP; > > return NF_ACCEPT; Looks good, we'll need to change some of the errno return codes in nf_ct_frag6_gather to 0 though for this to work, which should not be too hard ;)