Hi!
On Mon, Dec 31, 2018 at 12:28:52AM +0100, Phil Sutter wrote:
> Hi Florian,
>
> On Sun, Dec 30, 2018 at 08:10:28PM +0100, Florian Westphal wrote:
> > Phil Sutter <[email protected]> wrote:
> > > __nf_tables_dump_rules() stores the current idx value into cb->args[0]
> > > before returning to caller. With multiple chains present, cb->args[0] is
> > > therefore updated after each chain's rules have been traversed. This
> > > though causes the final nf_tables_dump_rules() run (which should return
> > > an skb->len of zero since no rules are left to dump) to continue dumping
> > > rules for each but the first chain. Fix this by moving the cb->args[0]
> > > update to nf_tables_dump_rules().
> > >
> > > With no final action to be performed anymore in
> > > __nf_tables_dump_rules(), drop 'out_unfinished' jump label and 'rc'
> > > variable - instead return the appropriate value directly.
> >
> > Looks good, but I think this is a bug too:
> >
> > list = rhltable_lookup(&table->chains_ht, ctx->chain,
> > nft_chain_ht_params);
> > if (!list)
> > goto done;
> >
> > I think this should move to next table instead.
>
> Hmm. Yes, assuming that specifying no table but only chain is a valid
> use-case, this should indeed continue with the next table. I'll send a
> v2 which includes that fix as well.
>
> > (Its not related to the bug at hand though).
>
> And not easy to trigger since all known users pass either both table and
> chain or none of them. :)
We can probably fix the problem that Florian reports via something
like the patch that I'm attaching? It is restricting the selecting
chain dump to work with ctx->table.
If fine with that, I would prefer to take your patch v1 to fix the
endless loop problem (ie. this patch).
Thanks!
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index e3ddd8e95e58..04d504a31e60 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -2350,7 +2350,7 @@ static int nf_tables_dump_rules(struct sk_buff *skb,
if (ctx && ctx->table && strcmp(ctx->table, table->name) != 0)
continue;
- if (ctx && ctx->chain) {
+ if (ctx && ctx->table && ctx->chain) {
struct rhlist_head *list, *tmp;
list = rhltable_lookup(&table->chains_ht, ctx->chain,
