Re: [syzbot] [netfilter?] possible deadlock in nf_tables_dumpreset_obj

[Florian Westphal]

syzbot <syzbot+ff16b505ec9152e5f448@xxxxxxxxxxxxxxxxxxxxxxxxx> wrote:
> syz.3.970/9330 is trying to acquire lock:
> ffff888012d4ccd8 (&nft_net->commit_mutex){+.+.}-{4:4}, at: nf_tables_dumpreset_obj+0x6f/0xa0 net/netfilter/nf_tables_api.c:8491
> 
> but task is already holding lock:
> ffff88802bce36f0 (nlk_cb_mutex-NETFILTER){+.+.}-{4:4}, at: __netlink_dump_start+0x150/0x990 net/netlink/af_netlink.c:2404
> 
> which lock already depends on the new lock.

I think this is a real bug:

CPU0: 'nft reset'.
CPU1: 'ipset list' (anything in ipset doing a netlink dump op)
CPU2: 'iptables-nft -A ... -m set ...'

... can result in:

CPU0                    CPU1                            CPU2
----                    ----                            ----
lock(nlk_cb_mutex-NETFILTER);
                        lock(nfnl_subsys_ipset);
                                                       lock(&nft_net->commit_mutex);
                        lock(nlk_cb_mutex-NETFILTER);
                                                       lock(nfnl_subsys_ipset);
lock(&nft_net->commit_mutex);

CPU0 is waiting for CPU2 to release transaction mutex.
CPU1 is waiting for CPU0 to release the netlink dump mutex
CPU2 is waiting for CPU1 to release the ipset subsys mutex

This bug was added when 'nft reset' started to grab the transaction
mutex from the dump callback path in nf_tables.

Not yet sure how to avoid it.
Maybe we could get rid of 'lock(nfnl_subsys_ipset);'
from the xt_set module call paths.

Or add a new lock (spinlock?) to protect the 'reset' object info
instead of using the transaction mutex.

I haven't given it much thought yet and will likely not
investigate further for the next two weeks.