| Message ID | 20160414152557.GA5338@shivani |
|---|---|
| State | Accepted |
| Delegated to: | Pablo Neira |
| Headers | show |
On Thu, Apr 14, 2016 at 08:55:58PM +0530, Shivani Bhardwaj 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 Shivani, thanks for following up on this. Would you also update extensions/libxt_NFQUEUE.t to add a test so we make sure this regression doesn't happen ever again? Thanks. -- 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
On Fri, Apr 15, 2016 at 12:44:05PM +0200, Pablo Neira Ayuso wrote: > Shivani, thanks for following up on this. > > Would you also update extensions/libxt_NFQUEUE.t to add a test so we > make sure this regression doesn't happen ever again? Just to clarify: No need to resend this patchset. A follow up standalone patch to update the test part would be sufficient. -- 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
On Fri, Apr 15, 2016 at 4:14 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Thu, Apr 14, 2016 at 08:55:58PM +0530, Shivani Bhardwaj 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 > > Shivani, thanks for following up on this. > > Would you also update extensions/libxt_NFQUEUE.t to add a test so we > make sure this regression doesn't happen ever again? > I just did that! :) Testing and sending out the patch. Thanks. > Thanks. -- 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
On Thu, Apr 14, 2016 at 08:55:58PM +0530, Shivani Bhardwaj 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. Applied, but please follow up with the test file update. Thanks Shivani. -- 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/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; } }
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 Suggested-by: Pablo Neira Ayuso <pablo@netfilter.org> Signed-off-by: Shivani Bhardwaj <shivanib134@gmail.com> --- extensions/libxt_NFQUEUE.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)