Message ID | 1391789931-3917-1-git-send-email-pablo@netfilter.org |
---|---|
State | Superseded |
Headers | show |
On Fri, Feb 07, 2014 at 05:18:51PM +0100, Pablo Neira Ayuso wrote: > Fix an oops if you use the "goto" operation, you will end up in > non-base chain with no counters and no default policy. While at > it, uninlined nft_chain_stats() since it doesn't make sense to > optimize a debugging facility that is rarely used. > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > --- > I noticed this when testing the compat layer. > > @Patrick: When working on this I noticed I accidentally has skipped > this patch: http://www.spinics.net/lists/netfilter-devel/msg29828.html > I have included this change in this fix. Sorry for that. Oops, unlined the wrong function... -- 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, Feb 07, 2014 at 05:23:30PM +0100, Pablo Neira Ayuso wrote: > On Fri, Feb 07, 2014 at 05:18:51PM +0100, Pablo Neira Ayuso wrote: > > Fix an oops if you use the "goto" operation, you will end up in > > non-base chain with no counters and no default policy. While at > > it, uninlined nft_chain_stats() since it doesn't make sense to > > optimize a debugging facility that is rarely used. > > > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > > --- > > I noticed this when testing the compat layer. > > > > @Patrick: When working on this I noticed I accidentally has skipped > > this patch: http://www.spinics.net/lists/netfilter-devel/msg29828.html > > I have included this change in this fix. Sorry for that. > > Oops, unlined the wrong function... I'm going to push this leftover fix to nf: http://patchwork.ozlabs.org/patch/308930/ Will resend a v2 to fix the goto issue. -- 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, Feb 07, 2014 at 05:26:58PM +0100, Pablo Neira Ayuso wrote: > On Fri, Feb 07, 2014 at 05:23:30PM +0100, Pablo Neira Ayuso wrote: > > On Fri, Feb 07, 2014 at 05:18:51PM +0100, Pablo Neira Ayuso wrote: > > > Fix an oops if you use the "goto" operation, you will end up in > > > non-base chain with no counters and no default policy. While at > > > it, uninlined nft_chain_stats() since it doesn't make sense to > > > optimize a debugging facility that is rarely used. > > > > > > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> > > > --- > > > I noticed this when testing the compat layer. > > > > > > @Patrick: When working on this I noticed I accidentally has skipped > > > this patch: http://www.spinics.net/lists/netfilter-devel/msg29828.html > > > I have included this change in this fix. Sorry for that. > > > > Oops, unlined the wrong function... > > I'm going to push this leftover fix to nf: > > http://patchwork.ozlabs.org/patch/308930/ > > Will resend a v2 to fix the goto issue. Thanks! I was also considering moving this to jump labels and enable the label on nft meta set trace. Not sure if its worth it though, will do some benchmarking at some point ... -- 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_tables_core.c b/net/netfilter/nf_tables_core.c index 0d879fc..bdee0ae 100644 --- a/net/netfilter/nf_tables_core.c +++ b/net/netfilter/nf_tables_core.c @@ -67,18 +67,24 @@ struct nft_jumpstack { int rulenum; }; -static inline void +static int nft_chain_stats(const struct nft_chain *this, const struct nft_pktinfo *pkt, struct nft_jumpstack *jumpstack, unsigned int stackptr) { struct nft_stats __percpu *stats; const struct nft_chain *chain = stackptr ? jumpstack[0].chain : this; + /* You can reach this through goto chain */ + if (!(chain->flags & NFT_BASE_CHAIN)) + return NF_ACCEPT; + rcu_read_lock_bh(); stats = rcu_dereference(nft_base_chain(chain)->stats); __this_cpu_inc(stats->pkts); __this_cpu_add(stats->bytes, pkt->skb->len); rcu_read_unlock_bh(); + + return nft_base_chain(chain)->policy; } enum nft_trace { @@ -209,12 +215,11 @@ next_rule: rulenum = jumpstack[stackptr].rulenum; goto next_rule; } - nft_chain_stats(chain, pkt, jumpstack, stackptr); if (unlikely(pkt->skb->nf_trace)) nft_trace_packet(pkt, chain, ++rulenum, NFT_TRACE_POLICY); - return nft_base_chain(chain)->policy; + return nft_chain_stats(chain, pkt, jumpstack, stackptr); } EXPORT_SYMBOL_GPL(nft_do_chain);
Fix an oops if you use the "goto" operation, you will end up in non-base chain with no counters and no default policy. While at it, uninlined nft_chain_stats() since it doesn't make sense to optimize a debugging facility that is rarely used. Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org> --- I noticed this when testing the compat layer. @Patrick: When working on this I noticed I accidentally has skipped this patch: http://www.spinics.net/lists/netfilter-devel/msg29828.html I have included this change in this fix. Sorry for that. net/netfilter/nf_tables_core.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-)