public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/3] net/sched: teql: Enforce hierarchy placement
@ 2026-01-14 16:02 Jamal Hadi Salim
  2026-01-14 16:02 ` [PATCH net 1/3] net/sched: Enforce that teql can only be used as root qdisc Jamal Hadi Salim
                   ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-01-14 16:02 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, horms, andrew+netdev
  Cc: netdev, xiyou.wangcong, jiri, victor, km.kim1503, security,
	Jamal Hadi Salim

GangMin Kim <km.kim1503@gmail.com> managed to create a UAF on qfq by inserting
teql as a child qdisc and exploiting a qlen sync issue.
teql is not intended to be used as a child qdisc. Lets enforce that rule in
patch #1. Although patch #1 fixes the issue, we prevent another potential qlen
exploit in qfq in patch #2 by enforcing the child's active status is not
determined by inspecting the qlen. In patch #3 we add a tdc test case.


Jamal Hadi Salim (2):
  net/sched: Enforce that teql can only be used as root qdisc
  net/sched: qfq: Use cl_is_active to determine whether  class is active
    in qfq_rm_from_ag

Victor Nogueira (1):
  selftests/tc-testing: Try to add teql as a child qdisc

 net/sched/sch_qfq.c                           |  2 +-
 net/sched/sch_teql.c                          |  5 ++++
 .../tc-testing/tc-tests/qdiscs/teql.json      | 25 +++++++++++++++++++
 3 files changed, 31 insertions(+), 1 deletion(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net 1/3] net/sched: Enforce that teql can only be used as root qdisc
  2026-01-14 16:02 [PATCH net 0/3] net/sched: teql: Enforce hierarchy placement Jamal Hadi Salim
@ 2026-01-14 16:02 ` Jamal Hadi Salim
  2026-01-14 16:02 ` [PATCH net 2/3] net/sched: qfq: Use cl_is_active to determine whether class is active in qfq_rm_from_ag Jamal Hadi Salim
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-01-14 16:02 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, horms, andrew+netdev
  Cc: netdev, xiyou.wangcong, jiri, victor, km.kim1503, security,
	Jamal Hadi Salim

Design intent of teql is that it is only supposed to be used as root qdisc.
We need to check for that constraint.

Although not important, I will describe the scenario that unearthed this
issue for the curious.

GangMin Kim <km.kim1503@gmail.com> managed to concot a scenario as follows:

ROOT qdisc 1:0 (QFQ)
  ├── class 1:1 (weight=15, lmax=16384) netem with delay 6.4s
  └── class 1:2 (weight=1, lmax=1514) teql

GangMin sends a packet which is enqueued to 1:1 (netem).
Any invocation of dequeue by QFQ from this class will not return a packet
until after 6.4s. In the meantime, a second packet is sent and it lands on
1:2. teql's enqueue will return success and this will activate class 1:2.
Main issue is that teql only updates the parent visible qlen (sch->q.qlen)
at dequeue. Since QFQ will only call dequeue if peek succeeds (and teql's
peek always returns NULL), dequeue will never be called and thus the qlen
will remain as 0. With that in mind, when GangMin updates 1:2's lmax value,
the qfq_change_class calls qfq_deact_rm_from_agg. Since the child qdisc's
qlen was not incremented, qfq fails to deactivate the class, but still
frees its pointers from the aggregate. So when the first packet is
rescheduled after 6.4 seconds (netem's delay), a dangling pointer is
accessed causing GangMin's causing a UAF.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: GangMin Kim <km.kim1503@gmail.com>
Tested-by: Victor Nogueira <victor@mojatatu.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_teql.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/net/sched/sch_teql.c b/net/sched/sch_teql.c
index 8badec6d82a2..6e4bdaa876ed 100644
--- a/net/sched/sch_teql.c
+++ b/net/sched/sch_teql.c
@@ -178,6 +178,11 @@ static int teql_qdisc_init(struct Qdisc *sch, struct nlattr *opt,
 	if (m->dev == dev)
 		return -ELOOP;
 
+	if (sch->parent != TC_H_ROOT) {
+		NL_SET_ERR_MSG_MOD(extack, "teql can only be used as root");
+		return -EOPNOTSUPP;
+	}
+
 	q->m = m;
 
 	skb_queue_head_init(&q->q);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net 2/3] net/sched: qfq: Use cl_is_active to determine whether class is active in qfq_rm_from_ag
  2026-01-14 16:02 [PATCH net 0/3] net/sched: teql: Enforce hierarchy placement Jamal Hadi Salim
  2026-01-14 16:02 ` [PATCH net 1/3] net/sched: Enforce that teql can only be used as root qdisc Jamal Hadi Salim
@ 2026-01-14 16:02 ` Jamal Hadi Salim
  2026-01-14 16:02 ` [PATCH net 3/3] selftests/tc-testing: Try to add teql as a child qdisc Jamal Hadi Salim
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-01-14 16:02 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, horms, andrew+netdev
  Cc: netdev, xiyou.wangcong, jiri, victor, km.kim1503, security,
	Jamal Hadi Salim

This is more of a preventive patch to make the code more consistent and
to prevent possible exploits that employ child qlen manipulations on qfq.
use cl_is_active instead of relying on the child qdisc's qlen to determine
class activation.

Fixes: 462dbc9101acd ("pkt_sched: QFQ Plus: fair-queueing service at DRR cost")
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_qfq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_qfq.c b/net/sched/sch_qfq.c
index f4013b547438..24ea023e4990 100644
--- a/net/sched/sch_qfq.c
+++ b/net/sched/sch_qfq.c
@@ -373,7 +373,7 @@ static void qfq_rm_from_agg(struct qfq_sched *q, struct qfq_class *cl)
 /* Deschedule class and remove it from its parent aggregate. */
 static void qfq_deact_rm_from_agg(struct qfq_sched *q, struct qfq_class *cl)
 {
-	if (cl->qdisc->q.qlen > 0) /* class is active */
+	if (cl_is_active(cl)) /* class is active */
 		qfq_deactivate_class(q, cl);
 
 	qfq_rm_from_agg(q, cl);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net 3/3] selftests/tc-testing: Try to add teql as a child qdisc
  2026-01-14 16:02 [PATCH net 0/3] net/sched: teql: Enforce hierarchy placement Jamal Hadi Salim
  2026-01-14 16:02 ` [PATCH net 1/3] net/sched: Enforce that teql can only be used as root qdisc Jamal Hadi Salim
  2026-01-14 16:02 ` [PATCH net 2/3] net/sched: qfq: Use cl_is_active to determine whether class is active in qfq_rm_from_ag Jamal Hadi Salim
@ 2026-01-14 16:02 ` Jamal Hadi Salim
  2026-01-15 19:16 ` [PATCH net 0/3] net/sched: teql: Enforce hierarchy placement Cong Wang
  2026-01-19 20:20 ` patchwork-bot+netdevbpf
  4 siblings, 0 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-01-14 16:02 UTC (permalink / raw)
  To: davem, edumazet, kuba, pabeni, horms, andrew+netdev
  Cc: netdev, xiyou.wangcong, jiri, victor, km.kim1503, security

From: Victor Nogueira <victor@mojatatu.com>

Add a selftest that attempts to add a teql qdisc as a qfq child.
Since teql _must_ be added as a root qdisc, the kernel should reject
this.

Signed-off-by: Victor Nogueira <victor@mojatatu.com>
---
 .../tc-testing/tc-tests/qdiscs/teql.json      | 25 +++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/teql.json b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/teql.json
index e5cc31f265f8..0179c57104ad 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/qdiscs/teql.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/qdiscs/teql.json
@@ -81,5 +81,30 @@
             "$TC qdisc del dev $DUMMY handle 1: root",
             "$IP link del dev $DUMMY"
         ]
+    },
+    {
+        "id": "124e",
+        "name": "Try to add teql as a child qdisc",
+        "category": [
+            "qdisc",
+            "ets",
+            "tbf"
+        ],
+        "plugins": {
+            "requires": [
+                "nsPlugin"
+            ]
+        },
+        "setup": [
+            "$TC qdisc add dev $DUMMY root handle 1: qfq",
+            "$TC class add dev $DUMMY parent 1: classid 1:1 qfq weight 15 maxpkt 16384"
+        ],
+        "cmdUnderTest": "$TC qdisc add dev $DUMMY parent 1:1 handle 2:1 teql0",
+        "expExitCode": "2",
+        "verifyCmd": "$TC -s -j qdisc ls dev $DUMMY parent 1:1",
+        "matchJSON": [],
+        "teardown": [
+            "$TC qdisc del dev $DUMMY root handle 1:"
+        ]
     }
 ]
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net 0/3] net/sched: teql: Enforce hierarchy placement
  2026-01-14 16:02 [PATCH net 0/3] net/sched: teql: Enforce hierarchy placement Jamal Hadi Salim
                   ` (2 preceding siblings ...)
  2026-01-14 16:02 ` [PATCH net 3/3] selftests/tc-testing: Try to add teql as a child qdisc Jamal Hadi Salim
@ 2026-01-15 19:16 ` Cong Wang
  2026-01-16 14:46   ` Jamal Hadi Salim
  2026-01-19 20:20 ` patchwork-bot+netdevbpf
  4 siblings, 1 reply; 7+ messages in thread
From: Cong Wang @ 2026-01-15 19:16 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: davem, edumazet, kuba, pabeni, horms, andrew+netdev, netdev, jiri,
	victor, km.kim1503, security

On Wed, Jan 14, 2026 at 8:03 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> GangMin Kim <km.kim1503@gmail.com> managed to create a UAF on qfq by inserting
> teql as a child qdisc and exploiting a qlen sync issue.
> teql is not intended to be used as a child qdisc. Lets enforce that rule in
> patch #1. Although patch #1 fixes the issue, we prevent another potential qlen
> exploit in qfq in patch #2 by enforcing the child's active status is not
> determined by inspecting the qlen. In patch #3 we add a tdc test case.

Is teql still used by anyone? If not, maybe it is time to remove it.

Regards,
Cong

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net 0/3] net/sched: teql: Enforce hierarchy placement
  2026-01-15 19:16 ` [PATCH net 0/3] net/sched: teql: Enforce hierarchy placement Cong Wang
@ 2026-01-16 14:46   ` Jamal Hadi Salim
  0 siblings, 0 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-01-16 14:46 UTC (permalink / raw)
  To: Cong Wang
  Cc: davem, edumazet, kuba, pabeni, horms, andrew+netdev, netdev, jiri,
	victor, km.kim1503, security

On Thu, Jan 15, 2026 at 2:16 PM Cong Wang <xiyou.wangcong@gmail.com> wrote:
>
> On Wed, Jan 14, 2026 at 8:03 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> >
> > GangMin Kim <km.kim1503@gmail.com> managed to create a UAF on qfq by inserting
> > teql as a child qdisc and exploiting a qlen sync issue.
> > teql is not intended to be used as a child qdisc. Lets enforce that rule in
> > patch #1. Although patch #1 fixes the issue, we prevent another potential qlen
> > exploit in qfq in patch #2 by enforcing the child's active status is not
> > determined by inspecting the qlen. In patch #3 we add a tdc test case.
>
> Is teql still used by anyone? If not, maybe it is time to remove it.
>

qfq is a better candidat - because it is a  magnet for hierachical set
ups by bounty hunters. And i have emailed the author several times
with zero responses.
Teql is stable and used by from what i have personally seen small
home/business routers to aggregate multiple isp links.

cheers,
jamal


> Regards,
> Cong

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net 0/3] net/sched: teql: Enforce hierarchy placement
  2026-01-14 16:02 [PATCH net 0/3] net/sched: teql: Enforce hierarchy placement Jamal Hadi Salim
                   ` (3 preceding siblings ...)
  2026-01-15 19:16 ` [PATCH net 0/3] net/sched: teql: Enforce hierarchy placement Cong Wang
@ 2026-01-19 20:20 ` patchwork-bot+netdevbpf
  4 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-01-19 20:20 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: davem, edumazet, kuba, pabeni, horms, andrew+netdev, netdev,
	xiyou.wangcong, jiri, victor, km.kim1503, security

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 14 Jan 2026 11:02:40 -0500 you wrote:
> GangMin Kim <km.kim1503@gmail.com> managed to create a UAF on qfq by inserting
> teql as a child qdisc and exploiting a qlen sync issue.
> teql is not intended to be used as a child qdisc. Lets enforce that rule in
> patch #1. Although patch #1 fixes the issue, we prevent another potential qlen
> exploit in qfq in patch #2 by enforcing the child's active status is not
> determined by inspecting the qlen. In patch #3 we add a tdc test case.
> 
> [...]

Here is the summary with links:
  - [net,1/3] net/sched: Enforce that teql can only be used as root qdisc
    https://git.kernel.org/netdev/net/c/50da4b9d07a7
  - [net,2/3] net/sched: qfq: Use cl_is_active to determine whether class is active in qfq_rm_from_ag
    https://git.kernel.org/netdev/net/c/d837fbee9245
  - [net,3/3] selftests/tc-testing: Try to add teql as a child qdisc
    https://git.kernel.org/netdev/net/c/2460f31e6e44

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-01-19 20:20 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-01-14 16:02 [PATCH net 0/3] net/sched: teql: Enforce hierarchy placement Jamal Hadi Salim
2026-01-14 16:02 ` [PATCH net 1/3] net/sched: Enforce that teql can only be used as root qdisc Jamal Hadi Salim
2026-01-14 16:02 ` [PATCH net 2/3] net/sched: qfq: Use cl_is_active to determine whether class is active in qfq_rm_from_ag Jamal Hadi Salim
2026-01-14 16:02 ` [PATCH net 3/3] selftests/tc-testing: Try to add teql as a child qdisc Jamal Hadi Salim
2026-01-15 19:16 ` [PATCH net 0/3] net/sched: teql: Enforce hierarchy placement Cong Wang
2026-01-16 14:46   ` Jamal Hadi Salim
2026-01-19 20:20 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox