[PATCH OLK-5.10 04/19] Revert "netfilter: nf_tables: fix chain binding transaction logic"

9 Oct 2023

Offering: HULK
hulk inclusion
category: bugfix
bugzilla: N/A
-------------------------------
This reverts commit 04982868dc7752bd35aa05e21a4d7216bac5b9b3.
Backport the dependency patch and then re-backport this patch.
Signed-off-by: Lu Wei luwei32@huawei.com
---
 include/net/netfilter/nf_tables.h | 21 +-------
 net/netfilter/nf_tables_api.c     | 84 +++++++++++------------------
 net/netfilter/nft_immediate.c     | 87 ++++---------------------------
 3 files changed, 41 insertions(+), 151 deletions(-)

diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
index 1a69a2201654..f0870b382678 100644
--- a/include/net/netfilter/nf_tables.h
+++ b/include/net/netfilter/nf_tables.h
@@ -908,10 +908,7 @@ static inline struct nft_userdata *nft_userdata(const struct nft_rule *rule)
    return (void *)&rule->data[rule->dlen];
 }
-void nft_rule_expr_activate(const struct nft_ctx *ctx, struct nft_rule *rule);
-void nft_rule_expr_deactivate(const struct nft_ctx *ctx, struct nft_rule *rule,
-			      enum nft_trans_phase phase);
-void nf_tables_rule_destroy(const struct nft_ctx *ctx, struct nft_rule *rule);
+void nf_tables_rule_release(const struct nft_ctx *ctx, struct nft_rule *rule);
static inline void nft_set_elem_update_expr(const struct nft_set_ext *ext,
    				    struct nft_regs *regs,
@@ -970,7 +967,6 @@ struct nft_chain {
 };
int nft_chain_validate(const struct nft_ctx *ctx, const struct nft_chain *chain);
-int nf_tables_bind_chain(const struct nft_ctx *ctx, struct nft_chain *chain);
enum nft_chain_types {
    NFT_CHAIN_T_DEFAULT = 0,
@@ -1007,17 +1003,11 @@ int nft_chain_validate_dependency(const struct nft_chain *chain,
 int nft_chain_validate_hooks(const struct nft_chain *chain,
                              unsigned int hook_flags);
-static inline bool nft_chain_binding(const struct nft_chain *chain)
-{
-	return chain->flags & NFT_CHAIN_BINDING;
-}
-
 static inline bool nft_chain_is_bound(struct nft_chain *chain)
 {
    return (chain->flags & NFT_CHAIN_BINDING) && chain->bound;
 }
-int nft_chain_add(struct nft_table *table, struct nft_chain *chain);
 void nft_chain_del(struct nft_chain *chain);
 void nf_tables_chain_destroy(struct nft_ctx *ctx);
@@ -1442,7 +1432,6 @@ struct nft_trans_rule {
    struct nft_rule			*rule;
    struct nft_flow_rule		*flow;
    u32				rule_id;
-	bool				bound;
 };
#define nft_trans_rule(trans)	\
@@ -1451,8 +1440,6 @@ struct nft_trans_rule {
    (((struct nft_trans_rule *)trans->data)->flow)
 #define nft_trans_rule_id(trans)	\
    (((struct nft_trans_rule *)trans->data)->rule_id)
-#define nft_trans_rule_bound(trans)	\
-	(((struct nft_trans_rule *)trans->data)->bound)
struct nft_trans_set {
    struct nft_set			*set;
@@ -1468,17 +1455,13 @@ struct nft_trans_set {
    (((struct nft_trans_set *)trans->data)->bound)
struct nft_trans_chain {
-	struct nft_chain		*chain;
    bool				update;
    char				*name;
    struct nft_stats __percpu	*stats;
    u8				policy;
-	bool				bound;
    u32				chain_id;
 };
-#define nft_trans_chain(trans)	\
-	(((struct nft_trans_chain *)trans->data)->chain)
 #define nft_trans_chain_update(trans)	\
    (((struct nft_trans_chain *)trans->data)->update)
 #define nft_trans_chain_name(trans)	\
@@ -1487,8 +1470,6 @@ struct nft_trans_chain {
    (((struct nft_trans_chain *)trans->data)->stats)
 #define nft_trans_chain_policy(trans)	\
    (((struct nft_trans_chain *)trans->data)->policy)
-#define nft_trans_chain_bound(trans)	\
-	(((struct nft_trans_chain *)trans->data)->bound)
 #define nft_trans_chain_id(trans)	\
    (((struct nft_trans_chain *)trans->data)->chain_id)
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index a37576a3dcf8..ea5418a36cf0 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -190,46 +190,6 @@ static void nft_set_trans_bind(const struct nft_ctx *ctx, struct nft_set *set)
    }
 }
-static void nft_chain_trans_bind(const struct nft_ctx *ctx, struct nft_chain *chain)
-{
-	struct net *net = ctx->net;
-	struct nft_trans *trans;
-
-	if (!nft_chain_binding(chain))
-		return;
-
-	list_for_each_entry_reverse(trans, &net->nft.commit_list, list) {
-		switch (trans->msg_type) {
-		case NFT_MSG_NEWCHAIN:
-			if (nft_trans_chain(trans) == chain)
-				nft_trans_chain_bound(trans) = true;
-			break;
-		case NFT_MSG_NEWRULE:
-			if (trans->ctx.chain == chain)
-				nft_trans_rule_bound(trans) = true;
-			break;
-		}
-	}
-}
-
-int nf_tables_bind_chain(const struct nft_ctx *ctx, struct nft_chain *chain)
-{
-	if (!nft_chain_binding(chain))
-		return 0;
-
-	if (nft_chain_binding(ctx->chain))
-		return -EOPNOTSUPP;
-
-	if (chain->bound)
-		return -EBUSY;
-
-	chain->bound = true;
-	chain->use++;
-	nft_chain_trans_bind(ctx, chain);
-
-	return 0;
-}
-
 static int nft_netdev_register_hooks(struct net *net,
    			     struct list_head *hook_list)
 {
@@ -353,9 +313,8 @@ static struct nft_trans *nft_trans_chain_add(struct nft_ctx *ctx, int msg_type)
    			ntohl(nla_get_be32(ctx->nla[NFTA_CHAIN_ID]));
    	}
    }
-	nft_trans_chain(trans) = ctx->chain;
-	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
+	list_add_tail(&trans->list, &ctx->net->nft.commit_list);
    return trans;
 }
@@ -373,7 +332,8 @@ static int nft_delchain(struct nft_ctx *ctx)
    return 0;
 }
-void nft_rule_expr_activate(const struct nft_ctx *ctx, struct nft_rule *rule)
+static void nft_rule_expr_activate(const struct nft_ctx *ctx,
+				   struct nft_rule *rule)
 {
    struct nft_expr *expr;
@@ -386,8 +346,9 @@ void nft_rule_expr_activate(const struct nft_ctx *ctx, struct nft_rule *rule)
    }
 }
-void nft_rule_expr_deactivate(const struct nft_ctx *ctx, struct nft_rule *rule,
-			      enum nft_trans_phase phase)
+static void nft_rule_expr_deactivate(const struct nft_ctx *ctx,
+				     struct nft_rule *rule,
+				     enum nft_trans_phase phase)
 {
    struct nft_expr *expr;
@@ -2020,7 +1981,7 @@ static int nft_basechain_init(struct nft_base_chain *basechain, u8 family,
    return 0;
 }
-int nft_chain_add(struct nft_table *table, struct nft_chain *chain)
+static int nft_chain_add(struct nft_table *table, struct nft_chain *chain)
 {
    int err;
@@ -3116,7 +3077,8 @@ static int nf_tables_getrule(struct net *net, struct sock *nlsk,
    return err;
 }
-void nf_tables_rule_destroy(const struct nft_ctx *ctx, struct nft_rule *rule)
+static void nf_tables_rule_destroy(const struct nft_ctx *ctx,
+				   struct nft_rule *rule)
 {
    struct nft_expr *expr, *next;
@@ -3133,7 +3095,7 @@ void nf_tables_rule_destroy(const struct nft_ctx *ctx, struct nft_rule *rule)
    kfree(rule);
 }
-static void nf_tables_rule_release(const struct nft_ctx *ctx, struct nft_rule *rule)
+void nf_tables_rule_release(const struct nft_ctx *ctx, struct nft_rule *rule)
 {
    nft_rule_expr_deactivate(ctx, rule, NFT_TRANS_RELEASE);
    nf_tables_rule_destroy(ctx, rule);
@@ -5533,6 +5495,7 @@ static int nf_tables_newsetelem(struct net *net, struct sock *nlsk,
 void nft_data_hold(const struct nft_data *data, enum nft_data_types type)
 {
    struct nft_chain *chain;
+	struct nft_rule *rule;
if (type == NFT_DATA_VERDICT) {
    	switch (data->verdict.code) {
@@ -5540,6 +5503,15 @@ void nft_data_hold(const struct nft_data *data, enum nft_data_types type)
    	case NFT_GOTO:
    		chain = data->verdict.chain;
    		chain->use++;
+
+			if (!nft_chain_is_bound(chain))
+				break;
+
+			chain->table->use++;
+			list_for_each_entry(rule, &chain->rules, list)
+				chain->use++;
+
+			nft_chain_add(chain->table, chain);
    		break;
    	}
    }
@@ -8205,7 +8177,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
    			kfree(nft_trans_chain_name(trans));
    			nft_trans_destroy(trans);
    		} else {
-				if (nft_trans_chain_bound(trans)) {
+				if (nft_chain_is_bound(trans->ctx.chain)) {
    				nft_trans_destroy(trans);
    				break;
    			}
@@ -8222,10 +8194,6 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action)
    		nft_trans_destroy(trans);
    		break;
    	case NFT_MSG_NEWRULE:
-			if (nft_trans_rule_bound(trans)) {
-				nft_trans_destroy(trans);
-				break;
-			}
    		trans->ctx.chain->use--;
    		list_del_rcu(&nft_trans_rule(trans)->list);
    		nft_rule_expr_deactivate(&trans->ctx,
@@ -8762,12 +8730,22 @@ static int nft_verdict_init(const struct nft_ctx *ctx, struct nft_data *data,
 static void nft_verdict_uninit(const struct nft_data *data)
 {
    struct nft_chain *chain;
+	struct nft_rule *rule;
switch (data->verdict.code) {
    case NFT_JUMP:
    case NFT_GOTO:
    	chain = data->verdict.chain;
    	chain->use--;
+
+		if (!nft_chain_is_bound(chain))
+			break;
+
+		chain->table->use--;
+		list_for_each_entry(rule, &chain->rules, list)
+			chain->use--;
+
+		nft_chain_del(chain);
    	break;
    }
 }
diff --git a/net/netfilter/nft_immediate.c b/net/netfilter/nft_immediate.c
index 9d4248898ce4..fcdbc5ed3f36 100644
--- a/net/netfilter/nft_immediate.c
+++ b/net/netfilter/nft_immediate.c
@@ -76,9 +76,11 @@ static int nft_immediate_init(const struct nft_ctx *ctx,
    	switch (priv->data.verdict.code) {
    	case NFT_JUMP:
    	case NFT_GOTO:
-			err = nf_tables_bind_chain(ctx, chain);
-			if (err < 0)
-				return err;
+			if (nft_chain_is_bound(chain)) {
+				err = -EBUSY;
+				goto err1;
+			}
+			chain->bound = true;
    		break;
    	default:
    		break;
@@ -96,31 +98,6 @@ static void nft_immediate_activate(const struct nft_ctx *ctx,
    			   const struct nft_expr *expr)
 {
    const struct nft_immediate_expr *priv = nft_expr_priv(expr);
-	const struct nft_data *data = &priv->data;
-	struct nft_ctx chain_ctx;
-	struct nft_chain *chain;
-	struct nft_rule *rule;
-
-	if (priv->dreg == NFT_REG_VERDICT) {
-		switch (data->verdict.code) {
-		case NFT_JUMP:
-		case NFT_GOTO:
-			chain = data->verdict.chain;
-			if (!nft_chain_binding(chain))
-				break;
-
-			chain_ctx = *ctx;
-			chain_ctx.chain = chain;
-
-			list_for_each_entry(rule, &chain->rules, list)
-				nft_rule_expr_activate(&chain_ctx, rule);
-
-			nft_clear(ctx->net, chain);
-			break;
-		default:
-			break;
-		}
-	}
return nft_data_hold(&priv->data, nft_dreg_to_type(priv->dreg));
 }
@@ -130,40 +107,6 @@ static void nft_immediate_deactivate(const struct nft_ctx *ctx,
    			     enum nft_trans_phase phase)
 {
    const struct nft_immediate_expr *priv = nft_expr_priv(expr);
-	const struct nft_data *data = &priv->data;
-	struct nft_ctx chain_ctx;
-	struct nft_chain *chain;
-	struct nft_rule *rule;
-
-	if (priv->dreg == NFT_REG_VERDICT) {
-		switch (data->verdict.code) {
-		case NFT_JUMP:
-		case NFT_GOTO:
-			chain = data->verdict.chain;
-			if (!nft_chain_binding(chain))
-				break;
-
-			chain_ctx = *ctx;
-			chain_ctx.chain = chain;
-
-			list_for_each_entry(rule, &chain->rules, list)
-				nft_rule_expr_deactivate(&chain_ctx, rule, phase);
-
-			switch (phase) {
-			case NFT_TRANS_PREPARE:
-				nft_deactivate_next(ctx->net, chain);
-				break;
-			default:
-				nft_chain_del(chain);
-				chain->bound = false;
-				chain->table->use--;
-				break;
-			}
-			break;
-		default:
-			break;
-		}
-	}
if (phase == NFT_TRANS_COMMIT)
    	return;
@@ -188,27 +131,15 @@ static void nft_immediate_destroy(const struct nft_ctx *ctx,
    case NFT_GOTO:
    	chain = data->verdict.chain;
-		if (!nft_chain_binding(chain))
-			break;
-
-		/* Rule construction failed, but chain is already bound:
-		 * let the transaction records release this chain and its rules.
-		 */
-		if (chain->bound) {
-			chain->use--;
+		if (!nft_chain_is_bound(chain))
    		break;
-		}
-		/* Rule has been deleted, release chain and its rules. */
    	chain_ctx = *ctx;
    	chain_ctx.chain = chain;
-		chain->use--;
-		list_for_each_entry_safe(rule, n, &chain->rules, list) {
-			chain->use--;
-			list_del(&rule->list);
-			nf_tables_rule_destroy(&chain_ctx, rule);
-		}
+		list_for_each_entry_safe(rule, n, &chain->rules, list)
+			nf_tables_rule_release(&chain_ctx, rule);
+
    	nf_tables_chain_destroy(&chain_ctx);
    	break;
    default:
-- 
2.34.1


    

2024

2023

2022

2021

2020

2019

[PATCH OLK-5.10 04/19] Revert "netfilter: nf_tables: fix chain binding transaction logic"