Re: [PATCH] NFQUEUE: Fix bug with order of fanout and bypass

[Florian Westphal <fw@strlen.de>]

Shivani Bhardwaj <shivanib134@gmail.com> wrote:
> NFQUEUE had a bug with the ordering of fanout and bypass options which
> was arising due to same and odd values for flags and bypass when used
> together. Because of this, during bitwise ANDing of flags and
> NFQ_FLAG_CPU_FANOUT, the value always evaluated to false (since
> NFQ_FLAG_CPU_FANOUT=0x02) and led to skipping of fanout option
> whenever it was used before bypass because then flags would be 1.
> 
> Before this patch,
> 
> $ sudo iptables -A FORWARD -j NFQUEUE -p TCP --sport 80 --queue-balance 0:3 --queue-cpu-fanout --queue-bypass
> 
> Chain FORWARD (policy ACCEPT)
> target     prot opt source               destination
> NFQUEUE    tcp  --  anywhere             anywhere             tcp spt:http NFQUEUE balance 0:3 bypass
> 
> After this patch,
> 
> Chain FORWARD (policy ACCEPT)
> target     prot opt source               destination
> NFQUEUE    tcp  --  anywhere             anywhere             tcp spt:http NFQUEUE balance 0:3 bypass cpu-fanout
 
> Closes bugzilla entry: http://bugzilla.netfilter.org/show_bug.cgi?id=939

Ugh, good catch!

> diff --git a/extensions/libxt_NFQUEUE.c b/extensions/libxt_NFQUEUE.c
> index 8115457..0b5becc 100644
> --- a/extensions/libxt_NFQUEUE.c
> +++ b/extensions/libxt_NFQUEUE.c
> @@ -99,7 +99,7 @@ static void NFQUEUE_parse_v2(struct xt_option_call *cb)
>  	NFQUEUE_parse_v1(cb);
>  	switch (cb->entry->id) {
>  	case O_QUEUE_BYPASS:
> -		info->bypass = 1;
> +		info->bypass |= NFQ_FLAG_BYPASS;
>  		break;

I don't like this mix of v2 and v3 layout.

Could you try to create an alternate patch that changes
NFQUEUE_parse_v3 to call NFQUEUE_parse_v1 and then add
	case O_QUEUE_BYPASS:
		info->bypass |= NFQ_FLAG_BYPASS;

to NFQUEUE_parse_v3?

I think that this would make it a bit clearer and
it also avoids the v3/v2/v1 stacking.

Thanks!