Unverified Commit 28186029 authored by openeuler-ci-bot's avatar openeuler-ci-bot Committed by Gitee
Browse files

!1436 Fix CVE-2023-3117 

Merge Pull Request from: @zhangjialin11 
 
Pablo Neira Ayuso (4):
  netfilter: nf_tables: incorrect error path handling with NFT_MSG_NEWRULE
  netfilter: nf_tables: fix chain binding transaction logic
  netfilter: nf_tables: add NFT_TRANS_PREPARE_ERROR to deal with bound set/chain
  netfilter: nf_tables: unbind non-anonymous set if rule construction fails

https://gitee.com/src-openeuler/kernel/issues/I7H68N 
 
Link:https://gitee.com/openeuler/kernel/pulls/1436

 

Reviewed-by: default avatarYue Haibing <yuehaibing@huawei.com>
parents ba0f2c9e dbc47736

include/net/netfilter/nf_tables.h

+22 −1
Original line number Diff line number Diff line
@@ -778,6 +778,7 @@ struct nft_expr_type { 


enum nft_trans_phase {

	NFT_TRANS_PREPARE,

	NFT_TRANS_PREPARE_ERROR,

	NFT_TRANS_ABORT,

	NFT_TRANS_COMMIT,

	NFT_TRANS_RELEASE
@@ -908,7 +909,10 @@ static inline struct nft_userdata *nft_userdata(const struct nft_rule *rule) 
	return (void *)&rule->data[rule->dlen];

}



void nf_tables_rule_release(const struct nft_ctx *ctx, struct nft_rule *rule);

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);



static inline void nft_set_elem_update_expr(const struct nft_set_ext *ext,

					    struct nft_regs *regs,
@@ -967,6 +971,8 @@ 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);

void nf_tables_unbind_chain(const struct nft_ctx *ctx, struct nft_chain *chain);



enum nft_chain_types {

	NFT_CHAIN_T_DEFAULT = 0,
@@ -1003,11 +1009,17 @@ 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);
@@ -1432,6 +1444,7 @@ struct nft_trans_rule { 
	struct nft_rule			*rule;

	struct nft_flow_rule		*flow;

	u32				rule_id;

	bool				bound;

};



#define nft_trans_rule(trans)	\
@@ -1440,6 +1453,8 @@ 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;
@@ -1455,13 +1470,17 @@ 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)	\
@@ -1470,6 +1489,8 @@ 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)

net/netfilter/nf_tables_api.c

+91 −35
Original line number Diff line number Diff line
@@ -168,7 +168,8 @@ static void nft_trans_destroy(struct nft_trans *trans) 
	kfree(trans);

}



static void nft_set_trans_bind(const struct nft_ctx *ctx, struct nft_set *set)

static void __nft_set_trans_bind(const struct nft_ctx *ctx, struct nft_set *set,

				 bool bind)

{

	struct net *net = ctx->net;

	struct nft_trans *trans;
@@ -180,14 +181,76 @@ static void nft_set_trans_bind(const struct nft_ctx *ctx, struct nft_set *set) 
		switch (trans->msg_type) {

		case NFT_MSG_NEWSET:

			if (nft_trans_set(trans) == set)

				nft_trans_set_bound(trans) = true;

				nft_trans_set_bound(trans) = bind;

			break;

		case NFT_MSG_NEWSETELEM:

			if (nft_trans_elem_set(trans) == set)

				nft_trans_elem_set_bound(trans) = true;

				nft_trans_elem_set_bound(trans) = bind;

			break;

		}

	}

}



static void nft_set_trans_bind(const struct nft_ctx *ctx, struct nft_set *set)

{

	return __nft_set_trans_bind(ctx, set, true);

}



static void nft_set_trans_unbind(const struct nft_ctx *ctx, struct nft_set *set)

{

	return __nft_set_trans_bind(ctx, set, false);

}



static void __nft_chain_trans_bind(const struct nft_ctx *ctx,

				   struct nft_chain *chain, bool bind)

{

	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) = bind;

			break;

		case NFT_MSG_NEWRULE:

			if (trans->ctx.chain == chain)

				nft_trans_rule_bound(trans) = bind;

			break;

		}

	}

}



static void nft_chain_trans_bind(const struct nft_ctx *ctx,

				 struct nft_chain *chain)

{

	__nft_chain_trans_bind(ctx, chain, true);

}



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;

}



void nf_tables_unbind_chain(const struct nft_ctx *ctx, struct nft_chain *chain)

{

	__nft_chain_trans_bind(ctx, chain, false);

}



static int nft_netdev_register_hooks(struct net *net,
@@ -313,8 +376,9 @@ 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);



	return trans;

}
@@ -332,8 +396,7 @@ static int nft_delchain(struct nft_ctx *ctx) 
	return 0;

}



static void nft_rule_expr_activate(const struct nft_ctx *ctx,

				   struct nft_rule *rule)

void nft_rule_expr_activate(const struct nft_ctx *ctx, struct nft_rule *rule)

{

	struct nft_expr *expr;
@@ -346,8 +409,7 @@ static void nft_rule_expr_activate(const struct nft_ctx *ctx, 
	}

}



static void nft_rule_expr_deactivate(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)

{

	struct nft_expr *expr;
@@ -1981,7 +2043,7 @@ static int nft_basechain_init(struct nft_base_chain *basechain, u8 family, 
	return 0;

}



static int nft_chain_add(struct nft_table *table, struct nft_chain *chain)

int nft_chain_add(struct nft_table *table, struct nft_chain *chain)

{

	int err;
@@ -3077,8 +3139,7 @@ static int nf_tables_getrule(struct net *net, struct sock *nlsk, 
	return err;

}



static void nf_tables_rule_destroy(const struct nft_ctx *ctx,

				   struct nft_rule *rule)

void nf_tables_rule_destroy(const struct nft_ctx *ctx, struct nft_rule *rule)

{

	struct nft_expr *expr, *next;
@@ -3095,7 +3156,7 @@ static void nf_tables_rule_destroy(const struct nft_ctx *ctx, 
	kfree(rule);

}



void nf_tables_rule_release(const struct nft_ctx *ctx, struct nft_rule *rule)

static 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);
@@ -3366,7 +3427,8 @@ static int nf_tables_newrule(struct net *net, struct sock *nlsk, 


	return 0;

err2:

	nf_tables_rule_release(&ctx, rule);

	nft_rule_expr_deactivate(&ctx, rule, NFT_TRANS_PREPARE_ERROR);

	nf_tables_rule_destroy(&ctx, rule);

err1:

	for (i = 0; i < n; i++) {

		if (info[i].ops) {
@@ -4495,6 +4557,15 @@ void nf_tables_deactivate_set(const struct nft_ctx *ctx, struct nft_set *set, 
			      enum nft_trans_phase phase)

{

	switch (phase) {

	case NFT_TRANS_PREPARE_ERROR:

		nft_set_trans_unbind(ctx, set);

		if (nft_set_is_anonymous(set))

			nft_deactivate_next(ctx->net, set);

		else

			list_del_rcu(&binding->list);



		set->use--;

		break;

	case NFT_TRANS_PREPARE:

		if (nft_set_is_anonymous(set))

			nft_deactivate_next(ctx->net, set);
@@ -5491,7 +5562,6 @@ 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) {
@@ -5499,15 +5569,6 @@ 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;

		}

	}
@@ -6431,6 +6492,7 @@ void nf_tables_deactivate_flowtable(const struct nft_ctx *ctx, 
				    enum nft_trans_phase phase)

{

	switch (phase) {

	case NFT_TRANS_PREPARE_ERROR:

	case NFT_TRANS_PREPARE:

	case NFT_TRANS_ABORT:

	case NFT_TRANS_RELEASE:
@@ -8173,7 +8235,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_chain_is_bound(trans->ctx.chain)) {

				if (nft_trans_chain_bound(trans)) {

					nft_trans_destroy(trans);

					break;

				}
@@ -8190,6 +8252,10 @@ 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,
@@ -8726,22 +8792,12 @@ 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;

	}

}

net/netfilter/nft_immediate.c

+81 −9
Original line number Diff line number Diff line
@@ -76,11 +76,9 @@ static int nft_immediate_init(const struct nft_ctx *ctx, 
		switch (priv->data.verdict.code) {

		case NFT_JUMP:

		case NFT_GOTO:

			if (nft_chain_is_bound(chain)) {

				err = -EBUSY;

				goto err1;

			}

			chain->bound = true;

			err = nf_tables_bind_chain(ctx, chain);

			if (err < 0)

				return err;

			break;

		default:

			break;
@@ -98,6 +96,31 @@ 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));

}
@@ -107,6 +130,43 @@ 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_ERROR:

				nf_tables_unbind_chain(ctx, chain);

				fallthrough;

			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;
@@ -131,15 +191,27 @@ static void nft_immediate_destroy(const struct nft_ctx *ctx, 
	case NFT_GOTO:

		chain = data->verdict.chain;



		if (!nft_chain_is_bound(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--;

			break;

		}



		/* Rule has been deleted, release chain and its rules. */

		chain_ctx = *ctx;

		chain_ctx.chain = chain;



		list_for_each_entry_safe(rule, n, &chain->rules, list)

			nf_tables_rule_release(&chain_ctx, rule);



		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);

		}

		nf_tables_chain_destroy(&chain_ctx);

		break;

	default: