Commit d472e985 authored by Pablo Neira Ayuso's avatar Pablo Neira Ayuso
Browse files

netfilter: nf_tables: register hooks last when adding new chain/flowtable 



Register hooks last when adding chain/flowtable to ensure that packets do
not walk over datastructure that is being released in the error path
without waiting for the rcu grace period.

Fixes: 91c7b38d ("netfilter: nf_tables: use new transaction infrastructure to handle chain")
Fixes: 3b49e2e9 ("netfilter: nf_tables: add flow table netlink frontend")
Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
parent 8762785f

net/netfilter/nf_tables_api.c

+40 −38
Original line number Diff line number Diff line
@@ -684,7 +684,8 @@ static int nft_delobj(struct nft_ctx *ctx, struct nft_object *obj) 
	return err;

}



static int nft_trans_flowtable_add(struct nft_ctx *ctx, int msg_type,

static struct nft_trans *

nft_trans_flowtable_add(struct nft_ctx *ctx, int msg_type,

		        struct nft_flowtable *flowtable)

{

	struct nft_trans *trans;
@@ -692,7 +693,7 @@ static int nft_trans_flowtable_add(struct nft_ctx *ctx, int msg_type, 
	trans = nft_trans_alloc(ctx, msg_type,

				sizeof(struct nft_trans_flowtable));

	if (trans == NULL)

		return -ENOMEM;

		return ERR_PTR(-ENOMEM);



	if (msg_type == NFT_MSG_NEWFLOWTABLE)

		nft_activate_next(ctx->net, flowtable);
@@ -701,22 +702,22 @@ static int nft_trans_flowtable_add(struct nft_ctx *ctx, int msg_type, 
	nft_trans_flowtable(trans) = flowtable;

	nft_trans_commit_list_add_tail(ctx->net, trans);



	return 0;

	return trans;

}



static int nft_delflowtable(struct nft_ctx *ctx,

			    struct nft_flowtable *flowtable)

{

	int err;

	struct nft_trans *trans;



	err = nft_trans_flowtable_add(ctx, NFT_MSG_DELFLOWTABLE, flowtable);

	if (err < 0)

		return err;

	trans = nft_trans_flowtable_add(ctx, NFT_MSG_DELFLOWTABLE, flowtable);

	if (IS_ERR(trans))

		return PTR_ERR(trans);



	nft_deactivate_next(ctx->net, flowtable);

	nft_use_dec(&ctx->table->use);



	return err;

	return 0;

}



static void __nft_reg_track_clobber(struct nft_regs_track *track, u8 dreg)
@@ -2504,19 +2505,15 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask, 
	RCU_INIT_POINTER(chain->blob_gen_0, blob);

	RCU_INIT_POINTER(chain->blob_gen_1, blob);



	err = nf_tables_register_hook(net, table, chain);

	if (err < 0)

		goto err_destroy_chain;



	if (!nft_use_inc(&table->use)) {

		err = -EMFILE;

		goto err_use;

		goto err_destroy_chain;

	}



	trans = nft_trans_chain_add(ctx, NFT_MSG_NEWCHAIN);

	if (IS_ERR(trans)) {

		err = PTR_ERR(trans);

		goto err_unregister_hook;

		goto err_trans;

	}



	nft_trans_chain_policy(trans) = NFT_CHAIN_POLICY_UNSET;
@@ -2524,17 +2521,22 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask, 
		nft_trans_chain_policy(trans) = policy;



	err = nft_chain_add(table, chain);

	if (err < 0) {

		nft_trans_destroy(trans);

		goto err_unregister_hook;

	}

	if (err < 0)

		goto err_chain_add;



	/* This must be LAST to ensure no packets are walking over this chain. */

	err = nf_tables_register_hook(net, table, chain);

	if (err < 0)

		goto err_register_hook;



	return 0;



err_unregister_hook:

err_register_hook:

	nft_chain_del(chain);

err_chain_add:

	nft_trans_destroy(trans);

err_trans:

	nft_use_dec_restore(&table->use);

err_use:

	nf_tables_unregister_hook(net, table, chain);

err_destroy_chain:

	nf_tables_chain_destroy(ctx);
@@ -8456,9 +8458,9 @@ static int nf_tables_newflowtable(struct sk_buff *skb, 
	u8 family = info->nfmsg->nfgen_family;

	const struct nf_flowtable_type *type;

	struct nft_flowtable *flowtable;

	struct nft_hook *hook, *next;

	struct net *net = info->net;

	struct nft_table *table;

	struct nft_trans *trans;

	struct nft_ctx ctx;

	int err;
@@ -8538,34 +8540,34 @@ static int nf_tables_newflowtable(struct sk_buff *skb, 
	err = nft_flowtable_parse_hook(&ctx, nla, &flowtable_hook, flowtable,

				       extack, true);

	if (err < 0)

		goto err4;

		goto err_flowtable_parse_hooks;



	list_splice(&flowtable_hook.list, &flowtable->hook_list);

	flowtable->data.priority = flowtable_hook.priority;

	flowtable->hooknum = flowtable_hook.num;



	trans = nft_trans_flowtable_add(&ctx, NFT_MSG_NEWFLOWTABLE, flowtable);

	if (IS_ERR(trans)) {

		err = PTR_ERR(trans);

		goto err_flowtable_trans;

	}



	/* This must be LAST to ensure no packets are walking over this flowtable. */

	err = nft_register_flowtable_net_hooks(ctx.net, table,

					       &flowtable->hook_list,

					       flowtable);

	if (err < 0) {

		nft_hooks_destroy(&flowtable->hook_list);

		goto err4;

	}



	err = nft_trans_flowtable_add(&ctx, NFT_MSG_NEWFLOWTABLE, flowtable);

	if (err < 0)

		goto err5;

		goto err_flowtable_hooks;



	list_add_tail_rcu(&flowtable->list, &table->flowtables);



	return 0;

err5:

	list_for_each_entry_safe(hook, next, &flowtable->hook_list, list) {

		nft_unregister_flowtable_hook(net, flowtable, hook);

		list_del_rcu(&hook->list);

		kfree_rcu(hook, rcu);

	}

err4:



err_flowtable_hooks:

	nft_trans_destroy(trans);

err_flowtable_trans:

	nft_hooks_destroy(&flowtable->hook_list);

err_flowtable_parse_hooks:

	flowtable->data.type->free(&flowtable->data);

err3:

	module_put(type->owner);