On Thu, Sep 28, 2023 at 07:46:30PM +0200, Florian Westphal wrote:
> Phil Sutter <phil@xxxxxx> 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.
>
> So I think this needs to be done under mutex.
OK, will do.
> > c.data = &dump_ctx;
> > @@ -6108,18 +6178,25 @@ static int nf_tables_getsetelem(struct sk_buff *skb,
> > if (!nla[NFTA_SET_ELEM_LIST_ELEMENTS])
> > return -EINVAL;
> >
> > + if (!try_module_get(THIS_MODULE))
> > + return -EINVAL;
> > + rcu_read_unlock();
> > + mutex_lock(&nft_net->commit_mutex);
> > + rcu_read_lock();
>
> Why do we need to regain the rcu read lock here?
> Are we tripping over a now bogus rcu_derefence check or is there
> another reason?
Yes, I got a lockdep warning because rhashtable_lookup() called from
nft_rhash_get() calls rht_dereference_rcu() which wants either ht->mutex
or RCU read lock held.
Cheers, Phil
