| Message ID | 1444389037-17285-1-git-send-email-fw@strlen.de |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
On Fri, Oct 09, 2015 at 01:10:37PM +0200, Florian Westphal wrote: > Usage of -prev seems buggy. While packet was out our hook cannot be > removed but we have no way to know if the previous one is still valid. > > So better not use ->prev at all. Since NF_REPEAT just asks to invoke > same hook function again, just do so, and continue with nf_interate > if we get an ACCEPT verdict. > > A side effect of this change is that if nf_reinject(NF_REPEAT) causes > another REPEAT we will now drop the skb instead of a kernel loop. > > However, NF_REPEAT loops would be a bug so this should not happen anyway. Good catch. Applied, thanks Florian. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c index 96777f9..253a995 100644 --- a/net/netfilter/nf_queue.c +++ b/net/netfilter/nf_queue.c @@ -192,10 +192,8 @@ void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict) nf_queue_entry_release_refs(entry); /* Continue traversal iff userspace said ok... */ - if (verdict == NF_REPEAT) { - elem = list_entry(elem->list.prev, struct nf_hook_ops, list); - verdict = NF_ACCEPT; - } + if (verdict == NF_REPEAT) + verdict = elem->hook(elem->priv, skb, &entry->state); if (verdict == NF_ACCEPT) { afinfo = nf_get_afinfo(entry->state.pf);
Usage of -prev seems buggy. While packet was out our hook cannot be removed but we have no way to know if the previous one is still valid. So better not use ->prev at all. Since NF_REPEAT just asks to invoke same hook function again, just do so, and continue with nf_interate if we get an ACCEPT verdict. A side effect of this change is that if nf_reinject(NF_REPEAT) causes another REPEAT we will now drop the skb instead of a kernel loop. However, NF_REPEAT loops would be a bug so this should not happen anyway. Signed-off-by: Florian Westphal <fw@strlen.de> --- Targetting -next: on -nf we need to s/elem->priv/elem/, else oops. I tested this with nf-queue.c from libnetfilter_queue, changed to issue NF_REPEAT + NFMARK=1 and -m mark --mark 1 -j ACCEPT -j NFQUEUE -> ping works, both rule counters increment.