* [PATCH nf,v2 1/3] netfilter: nf_tables: fix set->nelems counting with no NLM_F_EXCL
@ 2017-01-24 18:42 Pablo Neira Ayuso
2017-01-24 18:42 ` [PATCH nf,v2 2/3] netfilter: nf_tables: deconstify walk callback function Pablo Neira Ayuso
2017-01-24 18:42 ` [PATCH nf,v2 3/3] netfilter: nf_tables: bump set->ndeact on set flush Pablo Neira Ayuso
0 siblings, 2 replies; 3+ messages in thread
From: Pablo Neira Ayuso @ 2017-01-24 18:42 UTC (permalink / raw)
To: netfilter-devel
If the element exists and no NLM_F_EXCL is specified, do not bump
set->nelems, otherwise we leak one set element slot. This problem
amplifies if the set is full since the abort path always decrements the
counter for the -ENFILE case too, giving one spare extra slot.
Fix this by moving set->nelems update to nft_add_set_elem() after
successful element insertion. Moreover, remove the element if the set is
full so there is no need to rely on the abort path to undo things
anymore.
Fixes: c016c7e45ddf ("netfilter: nf_tables: honor NLM_F_EXCL flag in set element insertion")
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: http://patchwork.ozlabs.org/patch/716751/ attacks the problem from the
wrong angle since this doesn't catch more corner cases that trigger bugs.
This patch obsoletes this previous try.
net/netfilter/nf_tables_api.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index b84c7b25219b..831a9a16f563 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -3745,10 +3745,18 @@ static int nft_add_set_elem(struct nft_ctx *ctx, struct nft_set *set,
goto err5;
}
+ if (set->size &&
+ !atomic_add_unless(&set->nelems, 1, set->size + set->ndeact)) {
+ err = -ENFILE;
+ goto err6;
+ }
+
nft_trans_elem(trans) = elem;
list_add_tail(&trans->list, &ctx->net->nft.commit_list);
return 0;
+err6:
+ set->ops->remove(set, &elem);
err5:
kfree(trans);
err4:
@@ -3795,15 +3803,9 @@ static int nf_tables_newsetelem(struct net *net, struct sock *nlsk,
return -EBUSY;
nla_for_each_nested(attr, nla[NFTA_SET_ELEM_LIST_ELEMENTS], rem) {
- if (set->size &&
- !atomic_add_unless(&set->nelems, 1, set->size + set->ndeact))
- return -ENFILE;
-
err = nft_add_set_elem(&ctx, set, attr, nlh->nlmsg_flags);
- if (err < 0) {
- atomic_dec(&set->nelems);
+ if (err < 0)
break;
- }
}
return err;
}
--
2.1.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH nf,v2 2/3] netfilter: nf_tables: deconstify walk callback function
2017-01-24 18:42 [PATCH nf,v2 1/3] netfilter: nf_tables: fix set->nelems counting with no NLM_F_EXCL Pablo Neira Ayuso
@ 2017-01-24 18:42 ` Pablo Neira Ayuso
2017-01-24 18:42 ` [PATCH nf,v2 3/3] netfilter: nf_tables: bump set->ndeact on set flush Pablo Neira Ayuso
1 sibling, 0 replies; 3+ messages in thread
From: Pablo Neira Ayuso @ 2017-01-24 18:42 UTC (permalink / raw)
To: netfilter-devel
The flush operation needs to modify set and element objects, so let's
deconstify this.
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: no changes, rebase.
include/net/netfilter/nf_tables.h | 6 +++---
net/netfilter/nf_tables_api.c | 24 ++++++++++++------------
net/netfilter/nft_set_hash.c | 2 +-
net/netfilter/nft_set_rbtree.c | 2 +-
4 files changed, 17 insertions(+), 17 deletions(-)
diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 924325c46aab..7dfdb517f0be 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -207,9 +207,9 @@ struct nft_set_iter {
unsigned int skip;
int err;
int (*fn)(const struct nft_ctx *ctx,
- const struct nft_set *set,
+ struct nft_set *set,
const struct nft_set_iter *iter,
- const struct nft_set_elem *elem);
+ struct nft_set_elem *elem);
};
/**
@@ -301,7 +301,7 @@ struct nft_set_ops {
void (*remove)(const struct nft_set *set,
const struct nft_set_elem *elem);
void (*walk)(const struct nft_ctx *ctx,
- const struct nft_set *set,
+ struct nft_set *set,
struct nft_set_iter *iter);
unsigned int (*privsize)(const struct nlattr * const nla[]);
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 831a9a16f563..5bd0068320fb 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -3087,9 +3087,9 @@ static int nf_tables_delset(struct net *net, struct sock *nlsk,
}
static int nf_tables_bind_check_setelem(const struct nft_ctx *ctx,
- const struct nft_set *set,
+ struct nft_set *set,
const struct nft_set_iter *iter,
- const struct nft_set_elem *elem)
+ struct nft_set_elem *elem)
{
const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv);
enum nft_registers dreg;
@@ -3308,9 +3308,9 @@ struct nft_set_dump_args {
};
static int nf_tables_dump_setelem(const struct nft_ctx *ctx,
- const struct nft_set *set,
+ struct nft_set *set,
const struct nft_set_iter *iter,
- const struct nft_set_elem *elem)
+ struct nft_set_elem *elem)
{
struct nft_set_dump_args *args;
@@ -3322,7 +3322,7 @@ static int nf_tables_dump_set(struct sk_buff *skb, struct netlink_callback *cb)
{
struct net *net = sock_net(skb->sk);
u8 genmask = nft_genmask_cur(net);
- const struct nft_set *set;
+ struct nft_set *set;
struct nft_set_dump_args args;
struct nft_ctx ctx;
struct nlattr *nla[NFTA_SET_ELEM_LIST_MAX + 1];
@@ -3890,9 +3890,9 @@ static int nft_del_setelem(struct nft_ctx *ctx, struct nft_set *set,
}
static int nft_flush_set(const struct nft_ctx *ctx,
- const struct nft_set *set,
+ struct nft_set *set,
const struct nft_set_iter *iter,
- const struct nft_set_elem *elem)
+ struct nft_set_elem *elem)
{
struct nft_trans *trans;
int err;
@@ -3907,8 +3907,8 @@ static int nft_flush_set(const struct nft_ctx *ctx,
goto err1;
}
- nft_trans_elem_set(trans) = (struct nft_set *)set;
- nft_trans_elem(trans) = *((struct nft_set_elem *)elem);
+ nft_trans_elem_set(trans) = set;
+ nft_trans_elem(trans) = *elem;
list_add_tail(&trans->list, &ctx->net->nft.commit_list);
return 0;
@@ -5019,9 +5019,9 @@ static int nf_tables_check_loops(const struct nft_ctx *ctx,
const struct nft_chain *chain);
static int nf_tables_loop_check_setelem(const struct nft_ctx *ctx,
- const struct nft_set *set,
+ struct nft_set *set,
const struct nft_set_iter *iter,
- const struct nft_set_elem *elem)
+ struct nft_set_elem *elem)
{
const struct nft_set_ext *ext = nft_set_elem_ext(set, elem->priv);
const struct nft_data *data;
@@ -5045,7 +5045,7 @@ static int nf_tables_check_loops(const struct nft_ctx *ctx,
{
const struct nft_rule *rule;
const struct nft_expr *expr, *last;
- const struct nft_set *set;
+ struct nft_set *set;
struct nft_set_binding *binding;
struct nft_set_iter iter;
diff --git a/net/netfilter/nft_set_hash.c b/net/netfilter/nft_set_hash.c
index 1e20e2bbb6d9..e36069fb76ae 100644
--- a/net/netfilter/nft_set_hash.c
+++ b/net/netfilter/nft_set_hash.c
@@ -212,7 +212,7 @@ static void nft_hash_remove(const struct nft_set *set,
rhashtable_remove_fast(&priv->ht, &he->node, nft_hash_params);
}
-static void nft_hash_walk(const struct nft_ctx *ctx, const struct nft_set *set,
+static void nft_hash_walk(const struct nft_ctx *ctx, struct nft_set *set,
struct nft_set_iter *iter)
{
struct nft_hash *priv = nft_set_priv(set);
diff --git a/net/netfilter/nft_set_rbtree.c b/net/netfilter/nft_set_rbtree.c
index 08376e50f6cd..f06f55ee516d 100644
--- a/net/netfilter/nft_set_rbtree.c
+++ b/net/netfilter/nft_set_rbtree.c
@@ -221,7 +221,7 @@ static void *nft_rbtree_deactivate(const struct net *net,
}
static void nft_rbtree_walk(const struct nft_ctx *ctx,
- const struct nft_set *set,
+ struct nft_set *set,
struct nft_set_iter *iter)
{
const struct nft_rbtree *priv = nft_set_priv(set);
--
2.1.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH nf,v2 3/3] netfilter: nf_tables: bump set->ndeact on set flush
2017-01-24 18:42 [PATCH nf,v2 1/3] netfilter: nf_tables: fix set->nelems counting with no NLM_F_EXCL Pablo Neira Ayuso
2017-01-24 18:42 ` [PATCH nf,v2 2/3] netfilter: nf_tables: deconstify walk callback function Pablo Neira Ayuso
@ 2017-01-24 18:42 ` Pablo Neira Ayuso
1 sibling, 0 replies; 3+ messages in thread
From: Pablo Neira Ayuso @ 2017-01-24 18:42 UTC (permalink / raw)
To: netfilter-devel
Add missing set->ndeact update on each deactivated element from the set
flush path. Otherwise, sets with fixed size break after flush since
accounting breaks.
# nft add set x y { type ipv4_addr\; size 2\; }
# nft add element x y { 1.1.1.1 }
# nft add element x y { 1.1.1.2 }
# nft flush set x y
# nft add element x y { 1.1.1.1 }
<cmdline>:1:1-28: Error: Could not process rule: Too many open files in system
Fixes: 8411b6442e59 ("netfilter: nf_tables: support for set flushing")
Reported-by: Elise Lennion <elise.lennion@gmail.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
v2: no changes, rebase.
net/netfilter/nf_tables_api.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index 5bd0068320fb..1b913760f205 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -3906,6 +3906,7 @@ static int nft_flush_set(const struct nft_ctx *ctx,
err = -ENOENT;
goto err1;
}
+ set->ndeact++;
nft_trans_elem_set(trans) = set;
nft_trans_elem(trans) = *elem;
--
2.1.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-01-24 18:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-24 18:42 [PATCH nf,v2 1/3] netfilter: nf_tables: fix set->nelems counting with no NLM_F_EXCL Pablo Neira Ayuso
2017-01-24 18:42 ` [PATCH nf,v2 2/3] netfilter: nf_tables: deconstify walk callback function Pablo Neira Ayuso
2017-01-24 18:42 ` [PATCH nf,v2 3/3] netfilter: nf_tables: bump set->ndeact on set flush Pablo Neira Ayuso
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).