| Message ID | 20260927071520.3693598-1-nicoyip.dev@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [net] netfilter: nf_tables: serialize offload stats with callback unbind | expand |
On Sun, 27 Sep 2026 15:15:20 +0800 Chengfeng Ye wrote: > Rule GET and dump requests collect hardware statistics under RCU without > holding commit_mutex. Callback unbind removes entries from the base > chain's flow_block.cb_list and frees them immediately while holding > commit_mutex, so RCU does not protect the statistics iterator. Slow down. Please do not submit more than 8 patches for networking at a time. You can revise the already submitted patches but since you violated community guidelines we ask you not to submit any new fixes for a month.
On Mon, Sep 28, 2026 at 6:07 AM Jakub Kicinski <kuba@kernel.org> wrote: > > On Sun, 27 Sep 2026 15:15:20 +0800 Chengfeng Ye wrote: > > Rule GET and dump requests collect hardware statistics under RCU without > > holding commit_mutex. Callback unbind removes entries from the base > > chain's flow_block.cb_list and frees them immediately while holding > > commit_mutex, so RCU does not protect the statistics iterator. > > Slow down. Please do not submit more than 8 patches for networking > at a time. You can revise the already submitted patches but since you > violated community guidelines we ask you not to submit any new fixes > for a month. > -- > pv-ban: 1mo Hi Jakub, Understood, and I apologize for overwhelming the review queue. I had accumulated these fixes over time and submitted too many of them at once. Sorry for not carefully reading the submission guideline. I will not send new patches to the net or net-next and wait for these already sent patches to be consumed. Best regards, Chengfeng
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c index b59628e6240c..f234dc3f8d1e 100644 --- a/net/netfilter/nf_tables_api.c +++ b/net/netfilter/nf_tables_api.c @@ -3780,7 +3780,7 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net, const struct nft_table *table, const struct nft_chain *chain, const struct nft_rule *rule, u64 handle, - bool reset) + bool reset, bool commit_locked) { struct nlmsghdr *nlh; const struct nft_expr *expr, *next; @@ -3806,8 +3806,15 @@ static int nf_tables_fill_rule_info(struct sk_buff *skb, struct net *net, goto nla_put_failure; } - if (chain->flags & NFT_CHAIN_HW_OFFLOAD) - nft_flow_rule_stats(chain, rule); + if (chain->flags & NFT_CHAIN_HW_OFFLOAD) { + struct nftables_pernet *nft_net = nft_pernet(net); + + if (commit_locked || mutex_trylock(&nft_net->commit_mutex)) { + nft_flow_rule_stats(chain, rule); + if (!commit_locked) + mutex_unlock(&nft_net->commit_mutex); + } + } list = nla_nest_start_noflag(skb, NFTA_RULE_EXPRESSIONS); if (list == NULL) @@ -3864,7 +3871,7 @@ static void nf_tables_rule_notify(const struct nft_ctx *ctx, err = nf_tables_fill_rule_info(skb, ctx->net, ctx->portid, ctx->seq, event, flags, ctx->family, ctx->table, - ctx->chain, rule, handle, false); + ctx->chain, rule, handle, false, true); if (err < 0) { kfree_skb(skb); goto err; @@ -3924,7 +3931,8 @@ static int __nf_tables_dump_rules(struct sk_buff *skb, NFT_MSG_NEWRULE, NLM_F_MULTI | NLM_F_APPEND, table->family, - table, chain, rule, handle, ctx->reset) < 0) { + table, chain, rule, handle, + ctx->reset, false) < 0) { ret = 1; break; } @@ -4072,7 +4080,7 @@ nf_tables_getrule_single(u32 portid, const struct nfnl_info *info, err = nf_tables_fill_rule_info(skb2, net, portid, info->nlh->nlmsg_seq, NFT_MSG_NEWRULE, 0, - family, table, chain, rule, 0, reset); + family, table, chain, rule, 0, reset, false); if (err < 0) { kfree_skb(skb2); return ERR_PTR(err);
Rule GET and dump requests collect hardware statistics under RCU without holding commit_mutex. Callback unbind removes entries from the base chain's flow_block.cb_list and frees them immediately while holding commit_mutex, so RCU does not protect the statistics iterator. For example, a rule dump can load a block_cb in nft_setup_cb_call(), then a concurrent transaction can unbind the chain and free that callback in nft_flow_offload_unbind(). When the dump resumes, it dereferences the freed block_cb to invoke its callback. KASAN reported: BUG: KASAN: slab-use-after-free in nft_setup_cb_call.constprop.0+0x19d/0x200 Read of size 8 at addr ffff88810a64d420 by task poc/87 Call Trace: nft_setup_cb_call.constprop.0+0x19d/0x200 nft_flow_offload_cmd+0x23b/0x480 nft_flow_rule_stats+0x89/0x1d0 nf_tables_fill_rule_info+0x689/0x8e0 __nf_tables_dump_rules+0x256/0x940 nf_tables_dump_rules+0x762/0x9f0 netlink_dump+0x489/0x1140 Allocated by task 86: flow_block_cb_setup_simple+0x443/0x820 nft_block_offload_cmd+0x154/0x1e0 nft_flow_block_chain+0xf9/0x420 nft_flow_rule_offload_commit+0x448/0x5b0 Freed by task 88: kfree+0x131/0x3c0 nft_flow_offload_unbind+0x29c/0x400 nft_block_offload_cmd+0x166/0x1e0 nft_flow_block_chain+0xf9/0x420 nft_flow_rule_offload_commit+0x18d/0x5b0 Use mutex_trylock() to serialize read-side hardware statistics refresh with callback unbind. A blocking acquisition would sleep under RCU and reintroduce the commit_mutex -> nfnl_subsys_ipset -> nlk_cb_mutex -> commit_mutex lock dependency previously removed from reset dumps. On contention, skip the hardware refresh and retain the cached counters, as when the existing best-effort statistics request fails. Pass the caller's lock state so transaction notifications continue to refresh statistics under the mutex they already hold. Release an acquired mutex before dumping expressions, leaving rule lookup and callback order intact. Fixes: b72920f6e4a9 ("netfilter: nftables: counter hardware offload support") Cc: stable@vger.kernel.org Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com> --- net/netfilter/nf_tables_api.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-)