[PATCH net v1 0/2] net/sched: fix action batch deletion cleanup

[PATCH net v1 0/2] net/sched: fix action batch deletion cleanup

[Xuanqiang Luo]

From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>

Batched RTM_DELACTION requests can leak references to unprocessed actions
when deletion stops at a filter-bound action.

Patch 1 fixes the failure cleanup.

Patch 2 adds tc-testing regression coverage.

Failure reproduction (key output excerpts):

  python3 tdc.py -f /root/tc-testing/batch-delete.json

not ok 1 d710 - Release tail references after first action deletion fails
	Could not match regex pattern. Verify command output:
[...]
	 index 2 ref 2 bind 0
[...]
	 index 3 ref 2 bind 0

not ok 2 d711 - Release tail references after middle action deletion fails
	Could not match regex pattern. Verify command output:
[...]
	 index 3 ref 2 bind 0

not ok 3 d713 - Delete a tail action once after a failed batch
	Could not match regex pattern. Verify command output:
total acts 2
[...]
	 index 2 ref 1 bind 0

Xuanqiang Luo (2):
  net/sched: act_api: release tail references on DELACTION failure
  selftests: tc-testing: test action batch deletion failure cleanup

 net/sched/act_api.c                           |  11 +-
 .../tc-tests/actions/batch-delete.json        | 115 ++++++++++++++++++
 2 files changed, 123 insertions(+), 3 deletions(-)
 create mode 100644 tools/testing/selftests/tc-testing/tc-tests/actions/batch-delete.json


base-commit: 7f26a5e8040b4957ef4dbdfcde6cc7ba2db53937
-- 
2.43.0

[PATCH net v1 1/2] net/sched: act_api: release tail references on DELACTION failure

[Xuanqiang Luo]

From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>

A batched RTM_DELACTION request takes a temporary reference on each
action before attempting any deletion. tcf_action_delete() clears
each processed slot and drops its temporary reference before attempting
the deletion. If deletion fails, tca_action_gd() calls
tcf_action_put_many() to release the remaining references, but its
tcf_act_for_each_action() iterator stops at the first NULL slot.

When a batch stops at an action bound to a filter, this leaks a
reference on each subsequent action. A later delete of an unbound
action can then return success without removing it from the IDR.

Walk the full array in tcf_action_put_many() and skip NULL slots to
release the references held on the unprocessed actions.

Fixes: a0e947c9ccff ("net/sched: act_api: avoid non-contiguous action array")
Cc: stable@vger.kernel.org
Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
---
 net/sched/act_api.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index 37eced84dfa5f..86710a596a0fa 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -1223,11 +1223,16 @@ static int tcf_action_put(struct tc_action *p)
 
 static void tcf_action_put_many(struct tc_action *actions[])
 {
-	struct tc_action *a;
 	int i;
 
-	tcf_act_for_each_action(i, a, actions) {
-		const struct tc_action_ops *ops = a->ops;
+	/* Deletion may have cleared entries before failing. */
+	for (i = 0; i < TCA_ACT_MAX_PRIO; i++) {
+		struct tc_action *a = actions[i];
+		const struct tc_action_ops *ops;
+
+		if (!a)
+			continue;
+		ops = a->ops;
 		if (tcf_action_put(a))
 			module_put(ops->owner);
 	}
-- 
2.43.0

[PATCH net v1 2/2] selftests: tc-testing: test action batch deletion failure cleanup

[Xuanqiang Luo]

From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>

Add tests for cleanup after a batched RTM_DELACTION request fails at
a gact action bound to a filter. Check that subsequent actions retain
their original reference counts and that earlier successful deletions
are preserved.

Cover failures at the first and middle entries. Verify that a remaining
unbound action can be removed with one subsequent delete.

Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
---
 .../tc-tests/actions/batch-delete.json        | 115 ++++++++++++++++++
 1 file changed, 115 insertions(+)
 create mode 100644 tools/testing/selftests/tc-testing/tc-tests/actions/batch-delete.json

diff --git a/tools/testing/selftests/tc-testing/tc-tests/actions/batch-delete.json b/tools/testing/selftests/tc-testing/tc-tests/actions/batch-delete.json
new file mode 100644
index 0000000000000..ef7ca4a6775bc
--- /dev/null
+++ b/tools/testing/selftests/tc-testing/tc-tests/actions/batch-delete.json
@@ -0,0 +1,115 @@
+[
+    {
+        "id": "d710",
+        "name": "Release tail references after first action deletion fails",
+        "category": [
+            "actions",
+            "gact"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            [
+                "$TC actions flush action gact",
+                0,
+                1,
+                255
+            ],
+            "$TC qdisc add dev $DEV1 ingress",
+            "$TC actions add action pass index 1",
+            "$TC actions add action pass index 2",
+            "$TC actions add action pass index 3",
+            "$TC filter add dev $DEV1 protocol ip ingress u32 match u32 0 0 action gact index 1"
+        ],
+        "cmdUnderTest": "$TC actions del action gact index 1 action gact index 2 action gact index 3",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions ls action gact",
+        "matchPattern": "total acts 3\\b.*index 1 ref 2 bind 1\\b.*index 2 ref 1 bind 0\\b.*index 3 ref 1 bind 0\\b",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress",
+            [
+                "$TC actions flush action gact",
+                0,
+                1,
+                255
+            ]
+        ]
+    },
+    {
+        "id": "d711",
+        "name": "Release tail references after middle action deletion fails",
+        "category": [
+            "actions",
+            "gact"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            [
+                "$TC actions flush action gact",
+                0,
+                1,
+                255
+            ],
+            "$TC qdisc add dev $DEV1 ingress",
+            "$TC actions add action pass index 1",
+            "$TC actions add action pass index 2",
+            "$TC actions add action pass index 3",
+            "$TC filter add dev $DEV1 protocol ip ingress u32 match u32 0 0 action gact index 2"
+        ],
+        "cmdUnderTest": "$TC actions del action gact index 1 action gact index 2 action gact index 3",
+        "expExitCode": "255",
+        "verifyCmd": "$TC actions ls action gact",
+        "matchPattern": "total acts 2\\b.*index 2 ref 2 bind 1\\b.*index 3 ref 1 bind 0\\b",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress",
+            [
+                "$TC actions flush action gact",
+                0,
+                1,
+                255
+            ]
+        ]
+    },
+    {
+        "id": "d713",
+        "name": "Delete a tail action once after a failed batch",
+        "category": [
+            "actions",
+            "gact"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            [
+                "$TC actions flush action gact",
+                0,
+                1,
+                255
+            ],
+            "$TC qdisc add dev $DEV1 ingress",
+            "$TC actions add action pass index 1",
+            "$TC actions add action pass index 2",
+            "$TC filter add dev $DEV1 protocol ip ingress u32 match u32 0 0 action gact index 1"
+        ],
+        "cmdUnderTest": "$TC actions del action gact index 1 action gact index 2",
+        "expExitCode": "255",
+        "verifyCmd": "sh -c '$TC actions del action gact index 2 && $TC actions ls action gact'",
+        "matchPattern": "total acts 1\\b.*index 1 ref 2 bind 1\\b",
+        "matchCount": "1",
+        "teardown": [
+            "$TC qdisc del dev $DEV1 ingress",
+            [
+                "$TC actions flush action gact",
+                0,
+                1,
+                255
+            ]
+        ]
+    }
+]
-- 
2.43.0