netfilter: nf_tables: bogus EBUSY when deleting set after flush

Set deletion after flush coming in the same batch results in EBUSY. Add set use counter to track the number of references to this set from rules. We cannot rely on the list of bindings for this since such list is still populated from the preparation phase. Reported-by: Václav Zindulka <vaclav.zindulka@tlapnet.cz> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>

netfilter: nf_tables: bogus EBUSY when deleting set after flush
Set deletion after flush coming in the same batch results in EBUSY. Add set use counter to track the number of references to this set from rules. We cannot rely on the list of bindings for this since such list is still populated from the preparation phase. Reported-by: Václav Zindulka <vaclav.zindulka@tlapnet.cz> Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
Pablo Neira Ayuso
1 parent 40ba1d9b4d
Showing 5 changed files with 60 additions and 13 deletions Side-by-side Diff
include/net/netfilter/nf_tables.h
net/netfilter/nf_tables_api.c
net/netfilter/nft_dynset.c
net/netfilter/nft_lookup.c
net/netfilter/nft_objref.c
@@ -382,6 +382,7 @@
  * 	@dtype: data type (verdict or numeric type defined by userspace)
  * 	@objtype: object type (see NFT_OBJECT_* definitions)
  * 	@size: maximum set size
+ *	@use: number of rules references to this set
  * 	@nelems: number of elements
  * 	@ndeact: number of deactivated elements queued for removal
  *	@timeout: default timeout value in jiffies
@@ -407,6 +408,7 @@
 	u32				dtype;
 	u32				objtype;
 	u32				size;
+	u32				use;
 	atomic_t			nelems;
 	u32				ndeact;
 	u64				timeout;
@@ -467,6 +469,10 @@
 	u32				flags;
 };
  
+enum nft_trans_phase;
+void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
+			      struct nft_set_binding *binding,
+			      enum nft_trans_phase phase);
 int nf_tables_bind_set(const struct nft_ctx *ctx, struct nft_set *set,
 		       struct nft_set_binding *binding);
 void nf_tables_unbind_set(const struct nft_ctx *ctx, struct nft_set *set,
@@ -3672,6 +3672,9 @@
  
 static void nft_set_destroy(struct nft_set *set)
 {
+	if (WARN_ON(set->use > 0))
+		return;
+
 	set->ops->destroy(set);
 	module_put(to_set_type(set->ops)->owner);
 	kfree(set->name);
@@ -3712,7 +3715,7 @@
 		NL_SET_BAD_ATTR(extack, attr);
 		return PTR_ERR(set);
 	}
-	if (!list_empty(&set->bindings) ||
+	if (set->use ||
 	    (nlh->nlmsg_flags & NLM_F_NONREC && atomic_read(&set->nelems) > 0)) {
 		NL_SET_BAD_ATTR(extack, attr);
 		return -EBUSY;
@@ -3742,6 +3745,9 @@
 	struct nft_set_binding *i;
 	struct nft_set_iter iter;
  
+	if (set->use == UINT_MAX)
+		return -EOVERFLOW;
+
 	if (!list_empty(&set->bindings) && nft_set_is_anonymous(set))
 		return -EBUSY;
  
@@ -3769,6 +3775,7 @@
 	binding->chain = ctx->chain;
 	list_add_tail_rcu(&binding->list, &set->bindings);
 	nft_set_trans_bind(ctx, set);
+	set->use++;
  
 	return 0;
 }
@@ -3787,6 +3794,25 @@
 	}
 }
 EXPORT_SYMBOL_GPL(nf_tables_unbind_set);
+
+void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set,
+			      struct nft_set_binding *binding,
+			      enum nft_trans_phase phase)
+{
+	switch (phase) {
+	case NFT_TRANS_PREPARE:
+		set->use--;
+		return;
+	case NFT_TRANS_ABORT:
+	case NFT_TRANS_RELEASE:
+		set->use--;
+		/* fall through */
+	default:
+		nf_tables_unbind_set(ctx, set, binding,
+				     phase == NFT_TRANS_COMMIT);
+	}
+}
+EXPORT_SYMBOL_GPL(nf_tables_deactivate_set);
  
 void nf_tables_destroy_set(const struct nft_ctx *ctx, struct nft_set *set)
 {
@@ -240,11 +240,15 @@
 {
 	struct nft_dynset *priv = nft_expr_priv(expr);
  
-	if (phase == NFT_TRANS_PREPARE)
-		return;
+	nf_tables_deactivate_set(ctx, priv->set, &priv->binding, phase);
+}
  
-	nf_tables_unbind_set(ctx, priv->set, &priv->binding,
-			     phase == NFT_TRANS_COMMIT);
+static void nft_dynset_activate(const struct nft_ctx *ctx,
+				const struct nft_expr *expr)
+{
+	struct nft_dynset *priv = nft_expr_priv(expr);
+
+	priv->set->use++;
 }
  
 static void nft_dynset_destroy(const struct nft_ctx *ctx,
@@ -292,6 +296,7 @@
 	.eval		= nft_dynset_eval,
 	.init		= nft_dynset_init,
 	.destroy	= nft_dynset_destroy,
+	.activate	= nft_dynset_activate,
 	.deactivate	= nft_dynset_deactivate,
 	.dump		= nft_dynset_dump,
 };
@@ -127,11 +127,15 @@
 {
 	struct nft_lookup *priv = nft_expr_priv(expr);
  
-	if (phase == NFT_TRANS_PREPARE)
-		return;
+	nf_tables_deactivate_set(ctx, priv->set, &priv->binding, phase);
+}
  
-	nf_tables_unbind_set(ctx, priv->set, &priv->binding,
-			     phase == NFT_TRANS_COMMIT);
+static void nft_lookup_activate(const struct nft_ctx *ctx,
+				const struct nft_expr *expr)
+{
+	struct nft_lookup *priv = nft_expr_priv(expr);
+
+	priv->set->use++;
 }
  
 static void nft_lookup_destroy(const struct nft_ctx *ctx,
@@ -222,6 +226,7 @@
 	.size		= NFT_EXPR_SIZE(sizeof(struct nft_lookup)),
 	.eval		= nft_lookup_eval,
 	.init		= nft_lookup_init,
+	.activate	= nft_lookup_activate,
 	.deactivate	= nft_lookup_deactivate,
 	.destroy	= nft_lookup_destroy,
 	.dump		= nft_lookup_dump,
@@ -162,11 +162,15 @@
 {
 	struct nft_objref_map *priv = nft_expr_priv(expr);
  
-	if (phase == NFT_TRANS_PREPARE)
-		return;
+	nf_tables_deactivate_set(ctx, priv->set, &priv->binding, phase);
+}
  
-	nf_tables_unbind_set(ctx, priv->set, &priv->binding,
-			     phase == NFT_TRANS_COMMIT);
+static void nft_objref_map_activate(const struct nft_ctx *ctx,
+				    const struct nft_expr *expr)
+{
+	struct nft_objref_map *priv = nft_expr_priv(expr);
+
+	priv->set->use++;
 }
  
 static void nft_objref_map_destroy(const struct nft_ctx *ctx,
@@ -183,6 +187,7 @@
 	.size		= NFT_EXPR_SIZE(sizeof(struct nft_objref_map)),
 	.eval		= nft_objref_map_eval,
 	.init		= nft_objref_map_init,
+	.activate	= nft_objref_map_activate,
 	.deactivate	= nft_objref_map_deactivate,
 	.destroy	= nft_objref_map_destroy,
 	.dump		= nft_objref_map_dump,
...	...	@@ -382,6 +382,7 @@
382	382	* @dtype: data type (verdict or numeric type defined by userspace)
383	383	* @objtype: object type (see NFT_OBJECT_* definitions)
384	384	* @size: maximum set size
	385	+ * @use: number of rules references to this set
385	386	* @nelems: number of elements
386	387	* @ndeact: number of deactivated elements queued for removal
387	388	* @timeout: default timeout value in jiffies
...	...	@@ -407,6 +408,7 @@
407	408	u32 dtype;
408	409	u32 objtype;
409	410	u32 size;
	411	+ u32 use;
410	412	atomic_t nelems;
411	413	u32 ndeact;
412	414	u64 timeout;
...	...	@@ -467,6 +469,10 @@
467	469	u32 flags;
468	470	};
469	471
	472	+enum nft_trans_phase;
	473	+void nf_tables_deactivate_set(const struct nft_ctx ctx, struct nft_set set,
	474	+ struct nft_set_binding *binding,
	475	+ enum nft_trans_phase phase);
470	476	int nf_tables_bind_set(const struct nft_ctx ctx, struct nft_set set,
471	477	struct nft_set_binding *binding);
472	478	void nf_tables_unbind_set(const struct nft_ctx ctx, struct nft_set set,
...	...	@@ -3672,6 +3672,9 @@
3672	3672
3673	3673	static void nft_set_destroy(struct nft_set *set)
3674	3674	{
	3675	+ if (WARN_ON(set->use > 0))
	3676	+ return;
	3677	+
3675	3678	set->ops->destroy(set);
3676	3679	module_put(to_set_type(set->ops)->owner);
3677	3680	kfree(set->name);
...	...	@@ -3712,7 +3715,7 @@
3712	3715	NL_SET_BAD_ATTR(extack, attr);
3713	3716	return PTR_ERR(set);
3714	3717	}
3715		- if (!list_empty(&set->bindings) \|\|
	3718	+ if (set->use \|\|
3716	3719	(nlh->nlmsg_flags & NLM_F_NONREC && atomic_read(&set->nelems) > 0)) {
3717	3720	NL_SET_BAD_ATTR(extack, attr);
3718	3721	return -EBUSY;
...	...	@@ -3742,6 +3745,9 @@
3742	3745	struct nft_set_binding *i;
3743	3746	struct nft_set_iter iter;
3744	3747
	3748	+ if (set->use == UINT_MAX)
	3749	+ return -EOVERFLOW;
	3750	+
3745	3751	if (!list_empty(&set->bindings) && nft_set_is_anonymous(set))
3746	3752	return -EBUSY;
3747	3753
...	...	@@ -3769,6 +3775,7 @@
3769	3775	binding->chain = ctx->chain;
3770	3776	list_add_tail_rcu(&binding->list, &set->bindings);
3771	3777	nft_set_trans_bind(ctx, set);
	3778	+ set->use++;
3772	3779
3773	3780	return 0;
3774	3781	}
...	...	@@ -3787,6 +3794,25 @@
3787	3794	}
3788	3795	}
3789	3796	EXPORT_SYMBOL_GPL(nf_tables_unbind_set);
	3797	+
	3798	+void nf_tables_deactivate_set(const struct nft_ctx ctx, struct nft_set set,
	3799	+ struct nft_set_binding *binding,
	3800	+ enum nft_trans_phase phase)
	3801	+{
	3802	+ switch (phase) {
	3803	+ case NFT_TRANS_PREPARE:
	3804	+ set->use--;
	3805	+ return;
	3806	+ case NFT_TRANS_ABORT:
	3807	+ case NFT_TRANS_RELEASE:
	3808	+ set->use--;
	3809	+ /* fall through */
	3810	+ default:
	3811	+ nf_tables_unbind_set(ctx, set, binding,
	3812	+ phase == NFT_TRANS_COMMIT);
	3813	+ }
	3814	+}
	3815	+EXPORT_SYMBOL_GPL(nf_tables_deactivate_set);
3790	3816
3791	3817	void nf_tables_destroy_set(const struct nft_ctx ctx, struct nft_set set)
3792	3818	{
...	...	@@ -240,11 +240,15 @@
240	240	{
241	241	struct nft_dynset *priv = nft_expr_priv(expr);
242	242
243		- if (phase == NFT_TRANS_PREPARE)
244		- return;
	243	+ nf_tables_deactivate_set(ctx, priv->set, &priv->binding, phase);
	244	+}
245	245
246		- nf_tables_unbind_set(ctx, priv->set, &priv->binding,
247		- phase == NFT_TRANS_COMMIT);
	246	+static void nft_dynset_activate(const struct nft_ctx *ctx,
	247	+ const struct nft_expr *expr)
	248	+{
	249	+ struct nft_dynset *priv = nft_expr_priv(expr);
	250	+
	251	+ priv->set->use++;
248	252	}
249	253
250	254	static void nft_dynset_destroy(const struct nft_ctx *ctx,
...	...	@@ -292,6 +296,7 @@
292	296	.eval = nft_dynset_eval,
293	297	.init = nft_dynset_init,
294	298	.destroy = nft_dynset_destroy,
	299	+ .activate = nft_dynset_activate,
295	300	.deactivate = nft_dynset_deactivate,
296	301	.dump = nft_dynset_dump,
297	302	};
...	...	@@ -127,11 +127,15 @@
127	127	{
128	128	struct nft_lookup *priv = nft_expr_priv(expr);
129	129
130		- if (phase == NFT_TRANS_PREPARE)
131		- return;
	130	+ nf_tables_deactivate_set(ctx, priv->set, &priv->binding, phase);
	131	+}
132	132
133		- nf_tables_unbind_set(ctx, priv->set, &priv->binding,
134		- phase == NFT_TRANS_COMMIT);
	133	+static void nft_lookup_activate(const struct nft_ctx *ctx,
	134	+ const struct nft_expr *expr)
	135	+{
	136	+ struct nft_lookup *priv = nft_expr_priv(expr);
	137	+
	138	+ priv->set->use++;
135	139	}
136	140
137	141	static void nft_lookup_destroy(const struct nft_ctx *ctx,
...	...	@@ -222,6 +226,7 @@
222	226	.size = NFT_EXPR_SIZE(sizeof(struct nft_lookup)),
223	227	.eval = nft_lookup_eval,
224	228	.init = nft_lookup_init,
	229	+ .activate = nft_lookup_activate,
225	230	.deactivate = nft_lookup_deactivate,
226	231	.destroy = nft_lookup_destroy,
227	232	.dump = nft_lookup_dump,
...	...	@@ -162,11 +162,15 @@
162	162	{
163	163	struct nft_objref_map *priv = nft_expr_priv(expr);
164	164
165		- if (phase == NFT_TRANS_PREPARE)
166		- return;
	165	+ nf_tables_deactivate_set(ctx, priv->set, &priv->binding, phase);
	166	+}
167	167
168		- nf_tables_unbind_set(ctx, priv->set, &priv->binding,
169		- phase == NFT_TRANS_COMMIT);
	168	+static void nft_objref_map_activate(const struct nft_ctx *ctx,
	169	+ const struct nft_expr *expr)
	170	+{
	171	+ struct nft_objref_map *priv = nft_expr_priv(expr);
	172	+
	173	+ priv->set->use++;
170	174	}
171	175
172	176	static void nft_objref_map_destroy(const struct nft_ctx *ctx,
...	...	@@ -183,6 +187,7 @@
183	187	.size = NFT_EXPR_SIZE(sizeof(struct nft_objref_map)),
184	188	.eval = nft_objref_map_eval,
185	189	.init = nft_objref_map_init,
	190	+ .activate = nft_objref_map_activate,
186	191	.deactivate = nft_objref_map_deactivate,
187	192	.destroy = nft_objref_map_destroy,
188	193	.dump = nft_objref_map_dump,