From: Jamal Hadi Salim <jhs@mojatatu.com>
To: netdev@vger.kernel.org
Cc: Jamal Hadi Salim <jhs@mojatatu.com>,
Jiri Pirko <jiri@resnulli.us>,
"David S. Miller" <davem@davemloft.net>,
Eric Dumazet <edumazet@google.com>,
Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>,
Simon Horman <horms@kernel.org>,
Victor Nogueira <victor@mojatatu.com>,
stable@vger.kernel.org, Sashiko <sashiko-bot@kernel.org>,
hybris <hybris@mojatatu.ai>
Subject: [PATCH net] net/sched: cls_api: reclaim an empty proto on the error path
Date: Thu, 24 Sep 2026 04:32:49 -0400 [thread overview]
Message-ID: <QDISC-JCOT.v1.20260910090924@mojatatu.com> (raw)
Two racing tc filter add requests on the same chain/prio of an
unlocked classifier both run change() on the shared proto and both
can fail: the winner's tcf_chain_tp_delete_empty() attempt gives up
because the loser's handle is still in the idr, and the loser's
error path drops only its own reference without a second reclamation
attempt. The empty proto stays linked in the chain, holding the
chain reference, a block reference and the classifier module
reference until the chain or block is torn down.
Reclaim the proto on the error path of any failed request that holds
a proto reference. The reclamation is emptiness-gated:
tcf_chain_tp_delete_empty() unlinks the proto only when
delete_empty() admits it is empty, so a live shared proto is never
unlinked. A proto the request created is reclaimed unconditionally -
it is the only owner, so marking it for deletion is safe even without
a delete_empty callback. Classifiers without one (the check marks the
proto unconditionally) are rtnl-serialized, so the raced window this
guard closes cannot arise for them.
This is a follow-up to commit d4e359b3608a ("net/sched: cls_api: fix
teardown of an adopted proto on insert-race loss"), which stopped the
loser of the insert race from unlinking the winner's live proto but
left the empty-proto residual in place.
Conditions to recreate:
- CONFIG_NET_CLS_FLOWER=y; veth pair
- tc qdisc add dev veth0 ingress
- two concurrent `tc filter add dev veth0 ingress protocol ip pref 1
flower skip_sw ... action drop` (both fail in fl_hw_replace_filter
after publishing their handle in the idr); repeat in a loop
- an empty flower tp stays linked after both requests fail; visible
as a bare `filter protocol ip pref 1 flower chain 0` header in
`tc filter show` with no filter entries
- CAP_NET_ADMIN (namespace-local via unshare -Urn suffices)
Fixes: 8b64678e0af8 ("net: sched: refactor tp insert/delete for concurrent execution")
Reported-by: Sashiko (nipa) <sashiko-bot@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260805134049.927864-1-victor@mojatatu.com
Tested-by: hybris <hybris@mojatatu.ai>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
net/sched/cls_api.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index c47d2ee13641..a9f54988561f 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -2463,7 +2463,20 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
}
errout:
- if (err && tp_state == TP_CREATED)
+ if (err && !IS_ERR_OR_NULL(tp) &&
+ (tp_state == TP_CREATED || tp->ops->delete_empty))
+ /*
+ * The request is dropping its reference to tp. If it was
+ * the last user (the idr is empty now), reclaim the proto.
+ * A tp this request created is reclaimed unconditionally:
+ * it is the only owner, so marking it for deletion is
+ * safe. Otherwise only classifiers with a delete_empty
+ * callback are reclaimed -- the callback admits an empty
+ * proto only, so a live shared proto is never unlinked.
+ * Classifiers without one (deleting is set
+ * unconditionally) are rtnl-serialized, so the raced
+ * window this guard closes cannot arise for them.
+ */
tcf_chain_tp_delete_empty(chain, tp, rtnl_held, NULL);
errout_tp:
if (chain) {
--
2.43.0
next reply other threads:[~2026-09-24 8:33 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-09-24 8:32 Jamal Hadi Salim [this message]
2026-09-25 13:13 ` [PATCH net] net/sched: cls_api: reclaim an empty proto on the error path Simon Horman
2026-09-26 0:18 ` Jakub Kicinski
2026-09-26 9:49 ` Jamal Hadi Salim
2026-09-29 0:47 ` Jakub Kicinski
2026-09-29 8:42 ` Jamal Hadi Salim
2026-09-26 0:30 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=QDISC-JCOT.v1.20260910090924@mojatatu.com \
--to=jhs@mojatatu.com \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=horms@kernel.org \
--cc=hybris@mojatatu.ai \
--cc=jiri@resnulli.us \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=sashiko-bot@kernel.org \
--cc=stable@vger.kernel.org \
--cc=victor@mojatatu.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox