mirror of
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
synced 2026-08-30 13:23:02 -04:00
net/sched: cls_api: fix teardown of an adopted proto on insert-race loss
In tc_new_tfilter() the create branch sets tp_created = 1 before calling
tcf_chain_tp_insert_unique(). When the caller loses the race (another
request inserted a proto at the same chain/prio first), insert_unique()
destroys the caller's own tp_new and returns the winner's proto with an
extra reference. tp_created was never cleared, so the loser's errout
path treated the winner's live proto as its own and called
tcf_chain_tp_delete_empty() on it, silently unlinking an active
classifier that the winning request already advertised via
RTM_NEWTFILTER.
Track the outcome of the insert step in a single tri-state variable so
each errout path reacts correctly:
- TP_NOT_CREATED: no proto created; pursue the old path.
- TP_CREATED: proto inserted successfully; same code path as before.
- TP_NOT_OWNED: New - lost the insert race; tp is another request's proto
(chain ref already released by tp_new's destroy)
Both errout reactions are single expressions derived from the state.
This fix is motivated by the Sashiko's automated review of Patch
(net/sched: cls_api: Always acquire rtnl_lock when destroying locked
classifiers) [1][2]. The review identified the silent-unlink behaviour of
an adopted proto's teardown when a request loses the
tcf_chain_tp_insert_unique() race.
[1] https://sashiko.dev/#/patchset/20260801125632.360365-1-jhs%40mojatatu.com
[2] https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260801125632.360365-1-jhs%40mojatatu.com
Fixes: 8b64678e0a ("net: sched: refactor tp insert/delete for concurrent execution")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/20260801125632.360365-1-jhs%40mojatatu.com
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260801125632.360365-1-jhs%40mojatatu.com
Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Tested-by: Aohan Mei <henrymei@tencent.com>
Link: https://patch.msgid.link/20260805134049.927864-1-victor@mojatatu.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
committed by
Jakub Kicinski
parent
ee78f7f8d8
commit
d4e359b360
@@ -2248,6 +2248,12 @@ static bool is_ingress_or_clsact(struct tcf_block *block, struct Qdisc *q)
|
||||
return tcf_block_shared(block) || (q && !!(q->flags & TCQ_F_INGRESS));
|
||||
}
|
||||
|
||||
enum tcf_tp_insert_state {
|
||||
TP_NOT_CREATED = 0, /* did not create and insert a new tp */
|
||||
TP_CREATED, /* created and inserted a new tp */
|
||||
TP_NOT_OWNED, /* created a proto but failed to insert */
|
||||
};
|
||||
|
||||
static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
|
||||
struct netlink_ext_ack *extack)
|
||||
{
|
||||
@@ -2268,12 +2274,12 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
|
||||
unsigned long cl;
|
||||
void *fh;
|
||||
int err;
|
||||
int tp_created;
|
||||
enum tcf_tp_insert_state tp_state;
|
||||
bool rtnl_held = false;
|
||||
u32 flags;
|
||||
|
||||
replay:
|
||||
tp_created = 0;
|
||||
tp_state = TP_NOT_CREATED;
|
||||
|
||||
err = nlmsg_parse_deprecated(n, sizeof(*t), tca, TCA_MAX,
|
||||
rtm_tca_policy, extack);
|
||||
@@ -2395,13 +2401,15 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
|
||||
goto errout_tp;
|
||||
}
|
||||
|
||||
tp_created = 1;
|
||||
tp_state = TP_CREATED;
|
||||
tp = tcf_chain_tp_insert_unique(chain, tp_new, protocol, prio,
|
||||
rtnl_held);
|
||||
if (IS_ERR(tp)) {
|
||||
err = PTR_ERR(tp);
|
||||
goto errout_tp;
|
||||
}
|
||||
if (tp != tp_new)
|
||||
tp_state = TP_NOT_OWNED;
|
||||
} else {
|
||||
mutex_unlock(&chain->filter_chain_lock);
|
||||
}
|
||||
@@ -2455,13 +2463,13 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
|
||||
}
|
||||
|
||||
errout:
|
||||
if (err && tp_created)
|
||||
if (err && tp_state == TP_CREATED)
|
||||
tcf_chain_tp_delete_empty(chain, tp, rtnl_held, NULL);
|
||||
errout_tp:
|
||||
if (chain) {
|
||||
if (tp && !IS_ERR(tp))
|
||||
tcf_proto_put(tp, rtnl_held, NULL);
|
||||
if (!tp_created)
|
||||
if (tp_state == TP_NOT_CREATED)
|
||||
tcf_chain_put(chain);
|
||||
}
|
||||
tcf_block_release(q, block, rtnl_held);
|
||||
|
||||
Reference in New Issue
Block a user