Message ID | 1494320069-39638-1-git-send-email-gfree.wind@vip.163.com |
---|---|
State | Superseded, archived |
Delegated to: | David Miller |
Headers | show |
gfree.wind@vip.163.com <gfree.wind@vip.163.com> wrote: > When one netfilter rule or hook stoles the skb and return NF_STOLEN, > it means the skb is taken by the rule, and other modules should not > touch this skb ever. Maybe the skb is queued or freed directly by the > rule. > > Now uses the nf_hook instead of NF_HOOK to get the result of netfilter, > and check the return value of nf_hook. Only when its value equals 1, it > means the skb could go ahead. Or reset the skb as NULL. > > BTW, because vrf_rcv_finish is empty function, so needn't invoke it > even though nf_hook returns 1. Thats a bug then. The okfn (if called) takes ownership of skb and must free it eventually. Otherwise userspace queueing leaks skb on reinjection. (see nf_reinject() and its use of okfn()).
At 2017-05-09 17:21:02, "Florian Westphal" <fw@strlen.de> wrote: >gfree.wind@vip.163.com <gfree.wind@vip.163.com> wrote: >> When one netfilter rule or hook stoles the skb and return NF_STOLEN, >> it means the skb is taken by the rule, and other modules should not >> touch this skb ever. Maybe the skb is queued or freed directly by the >> rule. >> >> Now uses the nf_hook instead of NF_HOOK to get the result of netfilter, >> and check the return value of nf_hook. Only when its value equals 1, it >> means the skb could go ahead. Or reset the skb as NULL. >> >> BTW, because vrf_rcv_finish is empty function, so needn't invoke it >> even though nf_hook returns 1. > >Thats a bug then. > >The okfn (if called) takes ownership of skb and must free it eventually. >Otherwise userspace queueing leaks skb on reinjection. > >(see nf_reinject() and its use of okfn()). Thanks, I only thought about the stolen case like synproxy which would free the skb directly, and forget the userspace could reinject the skb. I would update the patch. Best Regards Feng
diff --git a/drivers/net/vrf.c b/drivers/net/vrf.c index ceda586..8960f44 100644 --- a/drivers/net/vrf.c +++ b/drivers/net/vrf.c @@ -998,7 +998,7 @@ static struct sk_buff *vrf_rcv_nfhook(u8 pf, unsigned int hook, { struct net *net = dev_net(dev); - if (NF_HOOK(pf, hook, net, NULL, skb, dev, NULL, vrf_rcv_finish) < 0) + if (nf_hook(pf, hook, net, NULL, skb, dev, NULL, vrf_rcv_finish) != 1) skb = NULL; /* kfree_skb(skb) handled by nf code */ return skb;