tca_action_gd() builds actions[] with one tcf_action_get_1() per nested
TCA_ACT_TAB entry. Each successful lookup takes its own reference to the
resolved action, so repeated TCA_ACT_INDEX entries in one request yield
the same pointer in multiple slots.
For RTM_DELACTION, tcf_action_delete() then consumes two references per
slot: one in tcf_action_put() and one in tcf_idr_delete_index(). A
duplicate therefore drives the refcount to zero mid-walk -- the delete
frees the action and removes its IDR slot -- and the next slot calls
tcf_action_put() on the freed action:
refcount_t: underflow; use-after-free.
WARNING: lib/refcount.c:87 at refcount_dec_not_one
refcount_dec_and_mutex_lock
__tcf_action_put
tca_action_gd
Three duplicate entries are enough. Reject a repeated action while a
delete batch is built, dropping the reference the extra lookup took, and
return -EINVAL. RTM_GETACTION balances its own references and keeps
accepting duplicate entries.
Conditions to recreate the bug:
tc actions add action gact index 100
tc actions delete action gact index 100 action gact index 100 \
action gact index 100
Fixes: 16af6067392c ("net: sched: implement reference counted action release")
Reported-by: Sashiko (gemini) <[email protected]>
Link:
https://sashiko.dev/#/patchset/[email protected]
Signed-off-by: Jamal Hadi Salim <[email protected]>
---
net/sched/act_api.c | 21 ++++++++++++++++++++-
1 file changed, 20 insertions(+), 1 deletion(-)
diff --git a/net/sched/act_api.c b/net/sched/act_api.c
index e45a63be397c..a9323c42a69a 100644
--- a/net/sched/act_api.c
+++ b/net/sched/act_api.c
@@ -2026,7 +2026,7 @@ static int
tca_action_gd(struct net *net, struct nlattr *nla, struct nlmsghdr *n,
u32 portid, int event, struct netlink_ext_ack *extack)
{
- int i, ret;
+ int i, j, ret;
struct nlattr *tb[TCA_ACT_MAX_PRIO + 1];
struct tc_action *act;
size_t attr_size = 0;
@@ -2051,6 +2051,25 @@ tca_action_gd(struct net *net, struct nlattr *nla,
struct nlmsghdr *n,
ret = PTR_ERR(act);
goto err;
}
+
+ /* A delete consumes two references per slot (tcf_action_put()
+ * and tcf_idr_delete_index()) but each entry takes one, so a
+ * repeated action would hit zero mid-walk. GET balances its
+ * own references and keeps accepting duplicates.
+ */
+ if (event == RTM_DELACTION) {
+ for (j = 0; j < i - 1; j++) {
+ if (actions[j] != act)
+ continue;
+
+ tcf_action_put(act);
+ NL_SET_ERR_MSG(extack,
+ "Duplicate TC action in a delete
batch");
+ ret = -EINVAL;
+ goto err;
+ }
+ }
+
attr_size += tcf_action_fill_size(act);
actions[i - 1] = act;
}
--
2.43.0
