Skip to content

Commit

Permalink
netfilter: nf_tables: reject loops from set element jump to chain
Browse files Browse the repository at this point in the history
Liping Zhang says:

"Users may add such a wrong nft rules successfully, which will cause an
endless jump loop:

  # nft add rule filter test tcp dport vmap {1: jump test}

This is because before we commit, the element in the current anonymous
set is inactive, so osp->walk will skip this element and miss the
validate check."

To resolve this problem, this patch passes the generation mask to the
walk function through the iter container structure depending on the code
path:

1) If we're dumping the elements, then we have to check if the element
   is active in the current generation. Thus, we check for the current
   bit in the genmask.

2) If we're checking for loops, then we have to check if the element is
   active in the next generation, as we're in the middle of a
   transaction. Thus, we check for the next bit in the genmask.

Based on original patch from Liping Zhang.

Reported-by: Liping Zhang <liping.zhang@spreadtrum.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Tested-by: Liping Zhang <liping.zhang@spreadtrum.com>
  • Loading branch information
Pablo Neira Ayuso committed Jun 15, 2016
1 parent a468440 commit 8588ac0
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 10 deletions.
1 change: 1 addition & 0 deletions include/net/netfilter/nf_tables.h
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,7 @@ struct nft_set_elem {

struct nft_set;
struct nft_set_iter {
u8 genmask;
unsigned int count;
unsigned int skip;
int err;
Expand Down
15 changes: 9 additions & 6 deletions net/netfilter/nf_tables_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -2951,6 +2951,7 @@ int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set,
goto bind;
}

iter.genmask = nft_genmask_next(ctx->net);
iter.skip = 0;
iter.count = 0;
iter.err = 0;
Expand Down Expand Up @@ -3192,12 +3193,13 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb)
if (nest == NULL)
goto nla_put_failure;

args.cb = cb;
args.skb = skb;
args.iter.skip = cb->args[0];
args.iter.count = 0;
args.iter.err = 0;
args.iter.fn = nf_tables_dump_setelem;
args.cb = cb;
args.skb = skb;
args.iter.genmask = nft_genmask_cur(ctx.net);
args.iter.skip = cb->args[0];
args.iter.count = 0;
args.iter.err = 0;
args.iter.fn = nf_tables_dump_setelem;
set->ops->walk(&ctx, set, &args.iter);

nla_nest_end(skb, nest);
Expand Down Expand Up @@ -4284,6 +4286,7 @@ static int nf_tables_check_loops(const struct nft_ctx *ctx,
binding->chain != chain)
continue;

iter.genmask = nft_genmask_next(ctx->net);
iter.skip = 0;
iter.count = 0;
iter.err = 0;
Expand Down
3 changes: 1 addition & 2 deletions net/netfilter/nft_hash.c
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ static void nft_hash_walk(const struct nft_ctx *ctx, const struct nft_set *set,
struct nft_hash_elem *he;
struct rhashtable_iter hti;
struct nft_set_elem elem;
u8 genmask = nft_genmask_cur(read_pnet(&set->pnet));
int err;

err = rhashtable_walk_init(&priv->ht, &hti, GFP_KERNEL);
Expand Down Expand Up @@ -218,7 +217,7 @@ static void nft_hash_walk(const struct nft_ctx *ctx, const struct nft_set *set,
goto cont;
if (nft_set_elem_expired(&he->ext))
goto cont;
if (!nft_set_elem_active(&he->ext, genmask))
if (!nft_set_elem_active(&he->ext, iter->genmask))
goto cont;

elem.priv = he;
Expand Down
3 changes: 1 addition & 2 deletions net/netfilter/nft_rbtree.c
Original file line number Diff line number Diff line change
Expand Up @@ -211,15 +211,14 @@ static void nft_rbtree_walk(const struct nft_ctx *ctx,
struct nft_rbtree_elem *rbe;
struct nft_set_elem elem;
struct rb_node *node;
u8 genmask = nft_genmask_cur(read_pnet(&set->pnet));

spin_lock_bh(&nft_rbtree_lock);
for (node = rb_first(&priv->root); node != NULL; node = rb_next(node)) {
rbe = rb_entry(node, struct nft_rbtree_elem, node);

if (iter->count < iter->skip)
goto cont;
if (!nft_set_elem_active(&rbe->ext, genmask))
if (!nft_set_elem_active(&rbe->ext, iter->genmask))
goto cont;

elem.priv = rbe;
Expand Down

0 comments on commit 8588ac0

Please sign in to comment.