Re: [nf PATCH v2 8/8] netfilter: nf_tables: Add locking for NFT_MSG_GETSETELEM_RESET requests

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Thu, Sep 28, 2023 at 07:46:30PM +0200, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > +static int nf_tables_dumpreset_set(struct sk_buff *skb,
> > +				   struct netlink_callback *cb)
> > +{
> > +	struct nftables_pernet *nft_net = nft_pernet(sock_net(skb->sk));
> > +	struct nft_set_dump_ctx *dump_ctx = cb->data;
> > +	int ret, skip = cb->args[0];
> > +
> > +	mutex_lock(&nft_net->commit_mutex);
> > +	ret = nf_tables_dump_set(skb, cb);
> > +	mutex_unlock(&nft_net->commit_mutex);
> > +
> > +	if (cb->args[0] > skip)
> > +		audit_log_nft_set_reset(dump_ctx->ctx.table, cb->seq,
> > +					cb->args[0] - skip);
> > +
> 
> Once commit_mutex is dropped, parallel user can
> delete table, and ctx.table references garbage.

This path should hold rcu read lock.

> So I think this needs to be done under mutex.

I think spinlock would be better, we would just spin for very little
time here for another thread to complete the reset, and the race is
fixed.

The use of commit_mutex here is confusing is really misleading to the
reader, this is also not the commit path.