[PATCH net] net/sched: cls_api: fix tp_created race losing existing tcf_proto

Aohan Mei <[email protected]>
Newsgroups org.kernel.vger.stable,org.kernel.vger.netdev
Message-ID <[email protected]>
tc_new_tfilter() attaches a filter to a chain. When no
tcf_proto (tp) exists for the given (protocol, prio), it creates one
with tcf_proto_create(), marks tp_created so the error path can clean
it up, and inserts it with tcf_chain_tp_insert_unique().

However, tcf_proto_create() can sleep, opening a race window in which a
concurrent thread may insert a tp with the same (protocol, prio). In
that case tcf_chain_tp_insert_unique() destroys tp_new and returns the
*existing* tp, but tp_created is never reset. If the request then fails
(e.g. kind mismatch), the errout path calls
tcf_chain_tp_delete_empty() on a tp this thread never created. For
classifiers without a delete_empty callback (all but cls_flower),
tcf_proto_check_delete() removes the tp unconditionally: a live tp and
all its filters are silently lost while the owner's change() still
reports success. The race is reachable because cls_flower on
ingress/clsact qdiscs runs without rtnl_lock and can interleave with
rtnl_lock-holding classifiers such as u32.

Fix this by making tcf_chain_tp_insert_unique() report through a new
"inserted" out-parameter whether tp_new was actually inserted, and
gate the errout delete_empty call on it instead of tp_created.
tp_created itself must stay set on this path: the chain reference was
consumed by the destroyed tp_new, and errout_tp relies on tp_created
to decide whether to tcf_chain_put(), so resetting it would underflow
the chain refcount.

Fixes: 8b64678e0af8 ("net: sched: refactor tp insert/delete for concurrent execution")
Cc: [email protected]
Reported-by: TencentOS Corvus AI <[email protected]>
Signed-off-by: Aohan Mei <[email protected]>
---
 net/sched/cls_api.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/net/sched/cls_api.c b/net/sched/cls_api.c
index fee4524ad..9ceb2b538 100644
--- a/net/sched/cls_api.c
+++ b/net/sched/cls_api.c
@@ -1937,7 +1937,8 @@ static struct tcf_proto *tcf_chain_tp_find(struct tcf_chain *chain,
 static struct tcf_proto *tcf_chain_tp_insert_unique(struct tcf_chain *chain,
 						    struct tcf_proto *tp_new,
 						    u32 protocol, u32 prio,
-						    bool rtnl_held)
+						    bool rtnl_held,
+						    bool *inserted)
 {
 	struct tcf_chain_info chain_info;
 	struct tcf_proto *tp;
@@ -1948,6 +1949,7 @@ static struct tcf_proto *tcf_chain_tp_insert_unique(struct tcf_chain *chain,
 	if (tcf_proto_exists_destroying(chain, tp_new)) {
 		mutex_unlock(&chain->filter_chain_lock);
 		tcf_proto_destroy(tp_new, rtnl_held, false, NULL);
+		*inserted = false;
 		return ERR_PTR(-EAGAIN);
 	}
 
@@ -1964,6 +1966,11 @@ static struct tcf_proto *tcf_chain_tp_insert_unique(struct tcf_chain *chain,
 		tp_new = ERR_PTR(err);
 	}
 
+	/* Tell the caller whether tp_new was actually inserted, or an
+	 * already existing tp is being returned instead.
+	 */
+	*inserted = !tp && !err;
+
 	return tp_new;
 }
 
@@ -2254,11 +2261,13 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 	void *fh;
 	int err;
 	int tp_created;
+	bool tp_inserted;
 	bool rtnl_held = false;
 	u32 flags;
 
 replay:
 	tp_created = 0;
+	tp_inserted = false;
 
 	err = nlmsg_parse_deprecated(n, sizeof(*t), tca, TCA_MAX,
 				     rtm_tca_policy, extack);
@@ -2382,7 +2391,7 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 
 		tp_created = 1;
 		tp = tcf_chain_tp_insert_unique(chain, tp_new, protocol, prio,
-						rtnl_held);
+						rtnl_held, &tp_inserted);
 		if (IS_ERR(tp)) {
 			err = PTR_ERR(tp);
 			goto errout_tp;
@@ -2440,7 +2449,13 @@ static int tc_new_tfilter(struct sk_buff *skb, struct nlmsghdr *n,
 	}
 
 errout:
-	if (err && tp_created)
+	/* Only delete a tp that we actually inserted ourselves. When
+	 * tcf_chain_tp_insert_unique() raced with a concurrent insertion
+	 * it returns the existing tp; tp_created must stay set then (the
+	 * chain reference was consumed by the destroyed tp_new), but the
+	 * existing tp must not be deleted.
+	 */
+	if (err && tp_inserted)
 		tcf_chain_tp_delete_empty(chain, tp, rtnl_held, NULL);
 errout_tp:
 	if (chain) {
-- 
2.50.1 (Apple Git-155)
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.