Commit d432f7bd authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge tag 'nf-24-04-04' of git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf 

Pablo Neira Ayuso says:

====================
Netfilter fixes for net

The following patchset contains Netfilter fixes for net:

Patch #1 unlike early commit path stage which triggers a call to abort,
         an explicit release of the batch is required on abort, otherwise
         mutex is released and commit_list remains in place.

Patch #2 release mutex after nft_gc_seq_end() in commit path, otherwise
         async GC worker could collect expired objects.

Patch #3 flush pending destroy work in module removal path, otherwise UaF
         is possible.

Patch #4 and #6 restrict the table dormant flag with basechain updates
	 to fix state inconsistency in the hook registration.

Patch #5 adds missing RCU read side lock to flowtable type to avoid races
	 with module removal.

* tag 'nf-24-04-04' of git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf:
  netfilter: nf_tables: discard table flag update with pending basechain deletion
  netfilter: nf_tables: Fix potential data-race in __nft_flowtable_type_get()
  netfilter: nf_tables: reject new basechain after table flag update
  netfilter: nf_tables: flush pending destroy work before exit_net release
  netfilter: nf_tables: release mutex after nft_gc_seq_end from abort path
  netfilter: nf_tables: release batch on table validation from abort path
====================

Link: https://lore.kernel.org/r/20240404104334.1627-1-pablo@netfilter.org


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents a66323e4 1bc83a01

net/netfilter/nf_tables_api.c

+34 −16
Original line number Diff line number Diff line
@@ -1209,10 +1209,11 @@ static bool nft_table_pending_update(const struct nft_ctx *ctx) 
		return true;



	list_for_each_entry(trans, &nft_net->commit_list, list) {

		if ((trans->msg_type == NFT_MSG_NEWCHAIN ||

		     trans->msg_type == NFT_MSG_DELCHAIN) &&

		    trans->ctx.table == ctx->table &&

		    nft_trans_chain_update(trans))

		if (trans->ctx.table == ctx->table &&

		    ((trans->msg_type == NFT_MSG_NEWCHAIN &&

		      nft_trans_chain_update(trans)) ||

		     (trans->msg_type == NFT_MSG_DELCHAIN &&

		      nft_is_base_chain(trans->ctx.chain))))

			return true;

	}
@@ -2449,6 +2450,9 @@ static int nf_tables_addchain(struct nft_ctx *ctx, u8 family, u8 genmask, 
		struct nft_stats __percpu *stats = NULL;

		struct nft_chain_hook hook = {};



		if (table->flags & __NFT_TABLE_F_UPDATE)

			return -EINVAL;



		if (flags & NFT_CHAIN_BINDING)

			return -EOPNOTSUPP;
@@ -8293,11 +8297,12 @@ static int nft_flowtable_parse_hook(const struct nft_ctx *ctx, 
	return err;

}



/* call under rcu_read_lock */

static const struct nf_flowtable_type *__nft_flowtable_type_get(u8 family)

{

	const struct nf_flowtable_type *type;



	list_for_each_entry(type, &nf_tables_flowtables, list) {

	list_for_each_entry_rcu(type, &nf_tables_flowtables, list) {

		if (family == type->family)

			return type;

	}
@@ -8309,9 +8314,13 @@ nft_flowtable_type_get(struct net *net, u8 family) 
{

	const struct nf_flowtable_type *type;



	rcu_read_lock();

	type = __nft_flowtable_type_get(family);

	if (type != NULL && try_module_get(type->owner))

	if (type != NULL && try_module_get(type->owner)) {

		rcu_read_unlock();

		return type;

	}

	rcu_read_unlock();



	lockdep_nfnl_nft_mutex_not_held();

#ifdef CONFIG_MODULES
@@ -10455,10 +10464,11 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action) 
	struct nft_trans *trans, *next;

	LIST_HEAD(set_update_list);

	struct nft_trans_elem *te;

	int err = 0;



	if (action == NFNL_ABORT_VALIDATE &&

	    nf_tables_validate(net) < 0)

		return -EAGAIN;

		err = -EAGAIN;



	list_for_each_entry_safe_reverse(trans, next, &nft_net->commit_list,

					 list) {
@@ -10650,12 +10660,7 @@ static int __nf_tables_abort(struct net *net, enum nfnl_abort_action action) 
		nf_tables_abort_release(trans);

	}



	if (action == NFNL_ABORT_AUTOLOAD)

		nf_tables_module_autoload(net);

	else

		nf_tables_module_autoload_cleanup(net);



	return 0;

	return err;

}



static int nf_tables_abort(struct net *net, struct sk_buff *skb,
@@ -10668,6 +10673,17 @@ static int nf_tables_abort(struct net *net, struct sk_buff *skb, 
	gc_seq = nft_gc_seq_begin(nft_net);

	ret = __nf_tables_abort(net, action);

	nft_gc_seq_end(nft_net, gc_seq);



	WARN_ON_ONCE(!list_empty(&nft_net->commit_list));



	/* module autoload needs to happen after GC sequence update because it

	 * temporarily releases and grabs mutex again.

	 */

	if (action == NFNL_ABORT_AUTOLOAD)

		nf_tables_module_autoload(net);

	else

		nf_tables_module_autoload_cleanup(net);



	mutex_unlock(&nft_net->commit_mutex);



	return ret;
@@ -11473,9 +11489,10 @@ static void __net_exit nf_tables_exit_net(struct net *net) 


	gc_seq = nft_gc_seq_begin(nft_net);



	if (!list_empty(&nft_net->commit_list) ||

	    !list_empty(&nft_net->module_list))

		__nf_tables_abort(net, NFNL_ABORT_NONE);

	WARN_ON_ONCE(!list_empty(&nft_net->commit_list));



	if (!list_empty(&nft_net->module_list))

		nf_tables_module_autoload_cleanup(net);



	__nft_release_tables(net);
@@ -11567,6 +11584,7 @@ static void __exit nf_tables_module_exit(void) 
	unregister_netdevice_notifier(&nf_tables_flowtable_notifier);

	nft_chain_filter_fini();

	nft_chain_route_fini();

	nf_tables_trans_destroy_flush_work();

	unregister_pernet_subsys(&nf_tables_net_ops);

	cancel_work_sync(&trans_gc_work);

	cancel_work_sync(&trans_destroy_work);