| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <69792899.050a0220.c9109.0023.GAE@google.com>
Date: Tue, 27 Jan 2026 13:05:29 -0800
From: syzbot <syzbot+ff16b505ec9152e5f448@...kaller.appspotmail.com>
To: brianwitte@...lfence.com
Cc: brianwitte@...lfence.com, linux-kernel@...r.kernel.org,
syzkaller-bugs@...glegroups.com
Subject: Re: test [PATCH nf-next] netfilter: nf_tables: use dedicated mutex
for reset operations
> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/netfilter/nf-next.git main
This crash does not have a reproducer. I cannot test it.
>
> From: Brian Witte <brianwitte@...lfence.com>
> Subject: netfilter: nf_tables: use dedicated mutex for reset operations
>
> Add a dedicated reset_mutex to serialize reset operations instead of
> reusing the commit_mutex. This fixes a circular locking dependency
> between commit_mutex, nfnl_subsys_ipset, and nlk_cb_mutex-NETFILTER
> that could lead to deadlock when nft reset, ipset list, and
> iptables-nft with set match run concurrently:
>
> CPU0 (nft reset): nlk_cb_mutex -> commit_mutex
> CPU1 (ipset list): nfnl_subsys_ipset -> nlk_cb_mutex
> CPU2 (iptables -m set): commit_mutex -> nfnl_subsys_ipset
>
> The reset_mutex only serializes concurrent reset operations to prevent
> counter underruns, which is all that's needed. Breaking the commit_mutex
> dependency in the dump-reset path eliminates the circular lock chain.
>
> Reported-by: syzbot+ff16b505ec9152e5f448@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=ff16b505ec9152e5f448
> Signed-off-by: Brian Witte <brianwitte@...lfence.com>
> ---
> include/net/netfilter/nf_tables.h | 1 +
> net/netfilter/nf_tables_api.c | 30 +++++++++++++++---------------
> 2 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/include/net/netfilter/nf_tables.h b/include/net/netfilter/nf_tables.h
> index 31906f90706e..85cdd93e564b 100644
> --- a/include/net/netfilter/nf_tables.h
> +++ b/include/net/netfilter/nf_tables.h
> @@ -1931,6 +1931,7 @@ struct nftables_pernet {
> struct list_head module_list;
> struct list_head notify_list;
> struct mutex commit_mutex;
> + struct mutex reset_mutex;
> u64 table_handle;
> u64 tstamp;
> unsigned int gc_seq;
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index be4924aeaf0e..c82b7875c49c 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -3907,13 +3907,12 @@ static int nf_tables_dumpreset_rules(struct sk_buff *skb,
> struct nftables_pernet *nft_net = nft_pernet(sock_net(skb->sk));
> int ret;
>
> - /* Mutex is held is to prevent that two concurrent dump-and-reset calls
> - * do not underrun counters and quotas. The commit_mutex is used for
> - * the lack a better lock, this is not transaction path.
> + /* Mutex is held to prevent that two concurrent dump-and-reset calls
> + * do not underrun counters and quotas.
> */
> - mutex_lock(&nft_net->commit_mutex);
> + mutex_lock(&nft_net->reset_mutex);
> ret = nf_tables_dump_rules(skb, cb);
> - mutex_unlock(&nft_net->commit_mutex);
> + mutex_unlock(&nft_net->reset_mutex);
>
> return ret;
> }
> @@ -4057,9 +4056,9 @@ static int nf_tables_getrule_reset(struct sk_buff *skb,
> if (!try_module_get(THIS_MODULE))
> return -EINVAL;
> rcu_read_unlock();
> - mutex_lock(&nft_net->commit_mutex);
> + mutex_lock(&nft_net->reset_mutex);
> skb2 = nf_tables_getrule_single(portid, info, nla, true);
> - mutex_unlock(&nft_net->commit_mutex);
> + mutex_unlock(&nft_net->reset_mutex);
> rcu_read_lock();
> module_put(THIS_MODULE);
>
> @@ -6346,7 +6345,7 @@ static int nf_tables_dumpreset_set(struct sk_buff *skb,
> struct nft_set_dump_ctx *dump_ctx = cb->data;
> int ret, skip = cb->args[0];
>
> - mutex_lock(&nft_net->commit_mutex);
> + mutex_lock(&nft_net->reset_mutex);
>
> ret = nf_tables_dump_set(skb, cb);
>
> @@ -6354,7 +6353,7 @@ static int nf_tables_dumpreset_set(struct sk_buff *skb,
> audit_log_nft_set_reset(dump_ctx->ctx.table, cb->seq,
> cb->args[0] - skip);
>
> - mutex_unlock(&nft_net->commit_mutex);
> + mutex_unlock(&nft_net->reset_mutex);
>
> return ret;
> }
> @@ -6671,7 +6670,7 @@ static int nf_tables_getsetelem_reset(struct sk_buff *skb,
> if (!try_module_get(THIS_MODULE))
> return -EINVAL;
> rcu_read_unlock();
> - mutex_lock(&nft_net->commit_mutex);
> + mutex_lock(&nft_net->reset_mutex);
> rcu_read_lock();
>
> err = nft_set_dump_ctx_init(&dump_ctx, skb, info, nla, true);
> @@ -6690,7 +6689,7 @@ static int nf_tables_getsetelem_reset(struct sk_buff *skb,
>
> out_unlock:
> rcu_read_unlock();
> - mutex_unlock(&nft_net->commit_mutex);
> + mutex_unlock(&nft_net->reset_mutex);
> rcu_read_lock();
> module_put(THIS_MODULE);
>
> @@ -8552,9 +8551,9 @@ static int nf_tables_dumpreset_obj(struct sk_buff *skb,
> struct nftables_pernet *nft_net = nft_pernet(sock_net(skb->sk));
> int ret;
>
> - mutex_lock(&nft_net->commit_mutex);
> + mutex_lock(&nft_net->reset_mutex);
> ret = nf_tables_dump_obj(skb, cb);
> - mutex_unlock(&nft_net->commit_mutex);
> + mutex_unlock(&nft_net->reset_mutex);
>
> return ret;
> }
> @@ -8693,9 +8692,9 @@ static int nf_tables_getobj_reset(struct sk_buff *skb,
> if (!try_module_get(THIS_MODULE))
> return -EINVAL;
> rcu_read_unlock();
> - mutex_lock(&nft_net->commit_mutex);
> + mutex_lock(&nft_net->reset_mutex);
> skb2 = nf_tables_getobj_single(portid, info, nla, true);
> - mutex_unlock(&nft_net->commit_mutex);
> + mutex_unlock(&nft_net->reset_mutex);
> rcu_read_lock();
> module_put(THIS_MODULE);
>
> @@ -12194,6 +12193,7 @@ static int __net_init nf_tables_init_net(struct net *net)
> INIT_LIST_HEAD(&nft_net->module_list);
> INIT_LIST_HEAD(&nft_net->notify_list);
> mutex_init(&nft_net->commit_mutex);
> + mutex_init(&nft_net->reset_mutex);
> net->nft.base_seq = 1;
> nft_net->gc_seq = 0;
> nft_net->validate_state = NFT_VALIDATE_SKIP;
> --
> 2.47.3
Powered by blists - more mailing lists