Message ID | 20181023144716.19746-1-fw@strlen.de |
---|---|
State | Accepted |
Delegated to: | Pablo Neira |
Headers | show |
Series | [nf] netfilter: ipv6: fix oops when defragmenting locally generated fragments | expand |
On Tue, Oct 23, 2018 at 7:48 AM Florian Westphal <fw@strlen.de> wrote: > > Unlike ipv4 and normal ipv6 defrag, netfilter ipv6 defragmentation did > not save/restore skb->dst. > > This causes oops when handling locally generated ipv6 fragments, as > output path needs a valid dst. > > Reported-by: Maciej Żenczykowski <zenczykowski@gmail.com> > Fixes: 84379c9afe01 ("netfilter: ipv6: nf_defrag: drop skb dst before queueing") > Signed-off-by: Florian Westphal <fw@strlen.de> Reviewed-by: Eric Dumazet <edumazet@google.com>
On Tue, Oct 23, 2018 at 7:47 AM, Florian Westphal <fw@strlen.de> wrote: > Unlike ipv4 and normal ipv6 defrag, netfilter ipv6 defragmentation did > not save/restore skb->dst. > > This causes oops when handling locally generated ipv6 fragments, as > output path needs a valid dst. > > Reported-by: Maciej Żenczykowski <zenczykowski@gmail.com> > Fixes: 84379c9afe01 ("netfilter: ipv6: nf_defrag: drop skb dst before queueing") > Signed-off-by: Florian Westphal <fw@strlen.de> > --- > net/ipv6/netfilter/nf_conntrack_reasm.c | 13 +++++++++---- > 1 file changed, 9 insertions(+), 4 deletions(-) > > diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c > index 8f68a518d9db..f76bd4d15704 100644 > --- a/net/ipv6/netfilter/nf_conntrack_reasm.c > +++ b/net/ipv6/netfilter/nf_conntrack_reasm.c > @@ -587,11 +587,16 @@ int nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 user) > */ > ret = -EINPROGRESS; > if (fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) && > - fq->q.meat == fq->q.len && > - nf_ct_frag6_reasm(fq, skb, dev)) > - ret = 0; > - else > + fq->q.meat == fq->q.len) { > + unsigned long orefdst = skb->_skb_refdst; > + > + skb->_skb_refdst = 0UL; > + if (nf_ct_frag6_reasm(fq, skb, dev)) > + ret = 0; > + skb->_skb_refdst = orefdst; > + } else { > skb_dst_drop(skb); > + } > > out_unlock: > spin_unlock_bh(&fq->q.lock); > -- > 2.18.1 > I don't quite follow how this fixes things, but I'll trust you on it. (nor do I understand why only 4.9 LTS appears to crash with a null ptr deref) Thanks for the fix.
Maciej Żenczykowski <maze@google.com> wrote: > > ret = -EINPROGRESS; > > if (fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) && > > - fq->q.meat == fq->q.len && > > - nf_ct_frag6_reasm(fq, skb, dev)) > > - ret = 0; > > - else > > + fq->q.meat == fq->q.len) { > > + unsigned long orefdst = skb->_skb_refdst; > > + > > + skb->_skb_refdst = 0UL; > > + if (nf_ct_frag6_reasm(fq, skb, dev)) > > + ret = 0; > > + skb->_skb_refdst = orefdst; > > + } else { > > skb_dst_drop(skb); > > + } > > > > out_unlock: > > spin_unlock_bh(&fq->q.lock); > > -- > > 2.18.1 > > > > I don't quite follow how this fixes things, but I'll trust you on it. The problematic spot is skb_morph() in nf_ct_frag6_reasm(), when we hit this code path we take dst from a dst-less skb that got queued earlier. > (nor do I understand why only 4.9 LTS appears to crash with a null ptr deref) Newer kernels need nft or iptables rule that enables defrag, such as "ip6tables -A INPUT -m conntrack --ctstate NEW"; 4.9 still enables it by default.
On Tue, Oct 23, 2018 at 04:47:16PM +0200, Florian Westphal wrote: > Unlike ipv4 and normal ipv6 defrag, netfilter ipv6 defragmentation did > not save/restore skb->dst. > > This causes oops when handling locally generated ipv6 fragments, as > output path needs a valid dst. Applied, thanks!
diff --git a/net/ipv6/netfilter/nf_conntrack_reasm.c b/net/ipv6/netfilter/nf_conntrack_reasm.c index 8f68a518d9db..f76bd4d15704 100644 --- a/net/ipv6/netfilter/nf_conntrack_reasm.c +++ b/net/ipv6/netfilter/nf_conntrack_reasm.c @@ -587,11 +587,16 @@ int nf_ct_frag6_gather(struct net *net, struct sk_buff *skb, u32 user) */ ret = -EINPROGRESS; if (fq->q.flags == (INET_FRAG_FIRST_IN | INET_FRAG_LAST_IN) && - fq->q.meat == fq->q.len && - nf_ct_frag6_reasm(fq, skb, dev)) - ret = 0; - else + fq->q.meat == fq->q.len) { + unsigned long orefdst = skb->_skb_refdst; + + skb->_skb_refdst = 0UL; + if (nf_ct_frag6_reasm(fq, skb, dev)) + ret = 0; + skb->_skb_refdst = orefdst; + } else { skb_dst_drop(skb); + } out_unlock: spin_unlock_bh(&fq->q.lock);
Unlike ipv4 and normal ipv6 defrag, netfilter ipv6 defragmentation did not save/restore skb->dst. This causes oops when handling locally generated ipv6 fragments, as output path needs a valid dst. Reported-by: Maciej Żenczykowski <zenczykowski@gmail.com> Fixes: 84379c9afe01 ("netfilter: ipv6: nf_defrag: drop skb dst before queueing") Signed-off-by: Florian Westphal <fw@strlen.de> --- net/ipv6/netfilter/nf_conntrack_reasm.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)