Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> writes:
> On Mon, Oct 17, 2016 at 11:23:01AM -0400, Aaron Conole wrote:
>> Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> writes:
>>
>> > Make sure we skip the current hook from where the packet was enqueued,
>> > otherwise the packets gets enqueued over and over again.
>> >
>> > Fixes: e3b37f11e6e4 ("netfilter: replace list_head with single linked list")
>> > Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
>> > ---
>> > v2: Make sure next hook is non-null, otherwise we are at the end of the
>> > hook list and we can skip nf_iterate().
>> >
>> > net/netfilter/nf_queue.c | 3 ++-
>> > 1 file changed, 2 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c
>> > index 96964a0070e1..691e713d70f5 100644
>> > --- a/net/netfilter/nf_queue.c
>> > +++ b/net/netfilter/nf_queue.c
>> > @@ -185,8 +185,9 @@ void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict)
>> > }
>> >
>> > entry->state.thresh = INT_MIN;
>> > + hook_entry = rcu_dereference(hook_entry->next);
>> >
>> > - if (verdict == NF_ACCEPT) {
>> > + if (hook_entry && verdict == NF_ACCEPT) {
>> > next_hook:
>> > verdict = nf_iterate(skb, &entry->state, &hook_entry);
>> > }
>>
>> ACK. I thought switch case below could have a problem, but re-checked
>> the first nf_queue leg, and it seems okay.
>
> Argh, still not right. If we get a NF_QUEUE verdict to re-enqueue
> again, then hook_entry may become NULL.
>
> switch (verdict & NF_VERDICT_MASK) {
> case NF_ACCEPT:
> case NF_STOP:
> local_bh_disable();
> entry->state.okfn(entry->state.net, entry->state.sk, skb);
> local_bh_enable();
> break;
> case NF_QUEUE:
> RCU_INIT_POINTER(entry->state.hook_entries, hook_entry); <--
>
> Attaching new patch.
>
> From c1a731c68791bcd504a7fe5d28f5f0fd59d66118 Mon Sep 17 00:00:00 2001
> From: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> Date: Thu, 13 Oct 2016 08:14:03 +0200
> Subject: [PATCH nf,v3] netfilter: nf_queue: don't re-enter same hook on packet
> reinjection
>
> If the packet is accepted, we have to skip the current hook from where
> the packet was enqueued. Thus, we can emulate the previous
> list_for_each_entry_continue() behaviour happening from nf_reinject(),
> otherwise the packets gets enqueued over and over again.
>
> Fixes: e3b37f11e6e4 ("netfilter: replace list_head with single linked list")
> Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> ---
> net/netfilter/nf_queue.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/nf_queue.c b/net/netfilter/nf_queue.c
> index 96964a0070e1..0b5ac3c9c2bc 100644
> --- a/net/netfilter/nf_queue.c
> +++ b/net/netfilter/nf_queue.c
> @@ -187,8 +187,10 @@ void nf_reinject(struct nf_queue_entry *entry, unsigned int verdict)
> entry->state.thresh = INT_MIN;
>
> if (verdict == NF_ACCEPT) {
> - next_hook:
> - verdict = nf_iterate(skb, &entry->state, &hook_entry);
> + hook_entry = rcu_dereference(hook_entry->next);
> + if (hook_entry)
> +next_hook:
Should the above two lines be transposed to this?
next_hook:
if (hook_entry)
Sorry if I'm misunderstanding it. Too many special cases for my tiny
brain...
-Aaron
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
