Skip to content

Commit

Permalink
Merge tag 'nf-24-04-04' of git://git.kernel.org/pub/scm/linux/kernel/…
Browse files Browse the repository at this point in the history
…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: Jakub Kicinski <kuba@kernel.org>
  • Loading branch information
kuba-moo committed Apr 4, 2024
2 parents a66323e + 1bc83a0 commit d432f7b
Showing 1 changed file with 34 additions and 16 deletions.
50 changes: 34 additions & 16 deletions net/netfilter/nf_tables_api.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}
Expand All @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
Expand Down

0 comments on commit d432f7b

Please sign in to comment.