public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 0/3] Replace direct dequeue call with qdisc_dequeue_peeked
@ 2026-04-30 15:29 Jamal Hadi Salim
  2026-04-30 15:29 ` [PATCH net 1/3] net/sched: sch_red: Replace direct dequeue call with peek and qdisc_dequeue_peeked Jamal Hadi Salim
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-04-30 15:29 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, davem, kuba, edumazet, pabeni, horms, jiri,
	victor, pctammela, ghandatmanas, rakshitawasthi17, security


When sfb and red qdiscs have children (eg qfq qdisc) whose peek() callback is
qdisc_peek_dequeued(), we could get a kernel panic. When the parent of such
qdiscs (eg illustrated in patch #3 as tbf) wants to retrieve an skb from
its child (red/sfb in this case), it will do the following:
 1a. do a peek() - and when sensing there's an skb the child can offer, then
     - the child in this case(red/sfb) calls its child's (qfq) peek.
        qfq does the right thing and will return the gso_skb queue packet.
        Note: if there wasnt a gso_skb entry then qfq will store it there.
 1b. invoke a dequeue() on the child (red/sfb). And herein lies the problem.
     - red/sfb will call the child's dequeue() which will essentially just
       try to grab something of qfq's queue.

The right thing to do in #1b is to grab the skb off gso_skb queue.
This patchset fixes that issue by changing #1b to use qdisc_dequeue_peeked()
method instead.

Patch 1 fixes the issue for red qdisc. Patch 2 fixes it for sfb.
Patch 3 adds testcases for the two setups.

Jamal Hadi Salim (2):
  net/sched: sch_red: Replace direct dequeue call with peek and
    qdisc_dequeue_peeked
  net/sched: sch_sfb: Replace direct dequeue call with peek and
    qdisc_dequeue_peeked

Victor Nogueira (1):
  selftests/tc-testing: Add tests that force red and sfb to dequeue from
    child's gso_skb

 net/sched/sch_red.c                           |   2 +-
 net/sched/sch_sfb.c                           |   2 +-
 .../tc-testing/tc-tests/infra/qdiscs.json     | 148 ++++++++++++++++++
 3 files changed, 150 insertions(+), 2 deletions(-)

-- 
2.34.1


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

* [PATCH net 1/3] net/sched: sch_red: Replace direct dequeue call with peek and qdisc_dequeue_peeked
  2026-04-30 15:29 [PATCH net 0/3] Replace direct dequeue call with qdisc_dequeue_peeked Jamal Hadi Salim
@ 2026-04-30 15:29 ` Jamal Hadi Salim
  2026-04-30 15:41   ` Eric Dumazet
  2026-04-30 15:29 ` [PATCH net 2/3] net/sched: sch_sfb: " Jamal Hadi Salim
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-04-30 15:29 UTC (permalink / raw)
  To: netdev
  Cc: Jamal Hadi Salim, davem, kuba, edumazet, pabeni, horms, jiri,
	victor, pctammela, ghandatmanas, rakshitawasthi17, security

When red qdisc has children (eg qfq qdisc) whose peek() callback is
qdisc_peek_dequeued(), we could get a kernel panic. When the parent of such
qdiscs (eg illustrated in patch #3 as tbf) wants to retrieve an skb from
its child (red in this case), it will do the following:
 1a. do a peek() - and when sensing there's an skb the child can offer, then
     - the child in this case(red) calls its child's (qfq) peek.
        qfq does the right thing and will return the gso_skb queue packet.
        Note: if there wasnt a gso_skb entry then qfq will store it there.
 1b. invoke a dequeue() on the child (red). And herein lies the problem.
     - red will call the child's dequeue() which will essentially just
       try to grab something of qfq's queue.

[   78.667668][  T363] KASAN: null-ptr-deref in range [0x0000000000000048-0x000000000000004f]
[   78.667927][  T363] CPU: 1 UID: 0 PID: 363 Comm: ping Not tainted 7.1.0-rc1-00033-g46f74a3f7d57-dirty #790 PREEMPT(full)
[   78.668263][  T363] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[   78.668486][  T363] RIP: 0010:qfq_dequeue+0x446/0xc90 [sch_qfq]
[   78.668718][  T363] Code: 54 c0 e8 dd 90 00 f1 48 c7 c7 e0 03 54 c0 48 89 de e8 ce 90 00 f1 48 8d 7b 48 b8 ff ff 37 00 48 89 fa 48 c1 e0 2a 48 c1 ea 03 <80> 3c 02 00 74 05 e8 ef a1 e1 f1 48 8b 7b 48 48 8d 54 24 58 48 8d
[   78.669312][  T363] RSP: 0018:ffff88810de573e0 EFLAGS: 00010216
[   78.669533][  T363] RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000000
[   78.669790][  T363] RDX: 0000000000000009 RSI: 0000000000000004 RDI: 0000000000000048
[   78.670044][  T363] RBP: ffff888110dc4000 R08: ffffffffb1b0885a R09: fffffbfff6ba9078
[   78.670297][  T363] R10: 0000000000000003 R11: ffff888110e31c80 R12: 0000001880000000
[   78.670560][  T363] R13: ffff888110dc4150 R14: ffff888110dc42b8 R15: 0000000000000200
[   78.670814][  T363] FS:  00007f66a8f09c40(0000) GS:ffff888163428000(0000) knlGS:0000000000000000
[   78.671110][  T363] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   78.671324][  T363] CR2: 000055db4c6a30a8 CR3: 000000010da67000 CR4: 0000000000750ef0
[   78.671585][  T363] PKRU: 55555554
[   78.671713][  T363] Call Trace:
[   78.671843][  T363]  <TASK>
[   78.671936][  T363]  ? __pfx_qfq_dequeue+0x10/0x10 [sch_qfq]
[   78.672148][  T363]  ? __pfx__printk+0x10/0x10
[   78.672322][  T363]  ? srso_alias_return_thunk+0x5/0xfbef5
[   78.672496][  T363]  ? lockdep_hardirqs_on_prepare+0xa8/0x1a0
[   78.672706][  T363]  ? srso_alias_return_thunk+0x5/0xfbef5
[   78.672875][  T363]  ? trace_hardirqs_on+0x19/0x1a0
[   78.673047][  T363]  red_dequeue+0x65/0x270 [sch_red]
[   78.673217][  T363]  ? srso_alias_return_thunk+0x5/0xfbef5
[   78.673385][  T363]  tbf_dequeue.cold+0xb0/0x70c [sch_tbf]
[   78.673566][  T363]  __qdisc_run+0x169/0x1900

The right thing to do in #1b is to grab the skb off gso_skb queue.
This patchset fixes that issue by changing #1b to use qdisc_dequeue_peeked()
method instead.


Fixes: 77be155cba4e ("pkt_sched: Add peek emulation for non-work-conserving qdiscs.")
Reported-by: Manas <ghandatmanas@gmail.com>
Reported-by: Rakshit Awasthi <rakshitawasthi17@gmail.com>
Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>
---
 net/sched/sch_red.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_red.c b/net/sched/sch_red.c
index 432b8a3000a5..4d0e44a2e7c6 100644
--- a/net/sched/sch_red.c
+++ b/net/sched/sch_red.c
@@ -162,7 +162,7 @@ static struct sk_buff *red_dequeue(struct Qdisc *sch)
 	struct red_sched_data *q = qdisc_priv(sch);
 	struct Qdisc *child = q->qdisc;
 
-	skb = child->dequeue(child);
+	skb = qdisc_dequeue_peeked(child);
 	if (skb) {
 		qdisc_bstats_update(sch, skb);
 		qdisc_qstats_backlog_dec(sch, skb);
-- 
2.34.1


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

* [PATCH net 2/3] net/sched: sch_sfb: Replace direct dequeue call with peek and qdisc_dequeue_peeked
  2026-04-30 15:29 [PATCH net 0/3] Replace direct dequeue call with qdisc_dequeue_peeked Jamal Hadi Salim
  2026-04-30 15:29 ` [PATCH net 1/3] net/sched: sch_red: Replace direct dequeue call with peek and qdisc_dequeue_peeked Jamal Hadi Salim
@ 2026-04-30 15:29 ` Jamal Hadi Salim
  2026-04-30 15:42   ` Eric Dumazet
  2026-04-30 15:29 ` [PATCH net 3/3] selftests/tc-testing: Add tests that force red and sfb to dequeue from child's gso_skb Jamal Hadi Salim
  2026-05-02 18:20 ` [PATCH net 0/3] Replace direct dequeue call with qdisc_dequeue_peeked patchwork-bot+netdevbpf
  3 siblings, 1 reply; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-04-30 15:29 UTC (permalink / raw)
  To: netdev
  Cc: Victor Nogueria, davem, kuba, edumazet, pabeni, horms, jiri,
	pctammela, ghandatmanas, rakshitawasthi17, security

From: Victor Nogueria <victor@mojatatu.com>

When sfb has children (eg qfq qdisc) whose peek() callback is
qdisc_peek_dequeued(), we could get a kernel panic. When the parent of such
qdiscs (eg illustrated in patch #3 as tbf) wants to retrieve an skb from
its child (sfb in this case), it will do the following:
 1a. do a peek() - and when sensing there's an skb the child can offer, then
     - the child in this case(sfb) calls its child's (qfq) peek.
        qfq does the right thing and will return the gso_skb queue packet.
        Note: if there wasnt a gso_skb entry then qfq will store it there.
 1b. invoke a dequeue() on the child (sfb). And herein lies the problem.
     - sfb will call the child's dequeue() which will essentially just
       try to grab something of qfq's queue.

[  127.594489][  T453] KASAN: null-ptr-deref in range [0x0000000000000048-0x000000000000004f]
[  127.594741][  T453] CPU: 2 UID: 0 PID: 453 Comm: ping Not tainted 7.1.0-rc1-00035-gac961974495b-dirty #793 PREEMPT(full)
[  127.595059][  T453] Hardware name: Bochs Bochs, BIOS Bochs 01/01/2011
[  127.595254][  T453] RIP: 0010:qfq_dequeue+0x35c/0x1650 [sch_qfq]
[  127.595461][  T453] Code: 00 fc ff df 80 3c 02 00 0f 85 17 0e 00 00 4c 8d 73 48 48 89 9d b8 02 00 00 48 b8 00 00 00 00 00 fc ff df 4c 89 f2 48 c1 ea 03 <80> 3c 02 00 0f 85 76 0c 00 00 48 b8 00 00 00 00 00 fc ff df 4c 8b
[  127.596081][  T453] RSP: 0018:ffff88810e5af440 EFLAGS: 00010216
[  127.596337][  T453] RAX: dffffc0000000000 RBX: 0000000000000000 RCX: dffffc0000000000
[  127.596623][  T453] RDX: 0000000000000009 RSI: 0000001880000000 RDI: ffff888104fd82b0
[  127.596917][  T453] RBP: ffff888104fd8000 R08: ffff888104fd8280 R09: 1ffff110211893a3
[  127.597165][  T453] R10: 1ffff110211893a6 R11: 1ffff110211893a7 R12: 0000001880000000
[  127.597404][  T453] R13: ffff888104fd82b8 R14: 0000000000000048 R15: 0000000040000000
[  127.597644][  T453] FS:  00007fc380cbfc40(0000) GS:ffff88816f2a8000(0000) knlGS:0000000000000000
[  127.597956][  T453] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[  127.598160][  T453] CR2: 00005610aa9890a8 CR3: 000000010369e000 CR4: 0000000000750ef0
[  127.598390][  T453] PKRU: 55555554
[  127.598509][  T453] Call Trace:
[  127.598629][  T453]  <TASK>
[  127.598718][  T453]  ? mark_held_locks+0x40/0x70
[  127.598890][  T453]  ? srso_alias_return_thunk+0x5/0xfbef5
[  127.599053][  T453]  sfb_dequeue+0x88/0x4d0
[  127.599174][  T453]  ? ktime_get+0x137/0x230
[  127.599328][  T453]  ? srso_alias_return_thunk+0x5/0xfbef5
[  127.599480][  T453]  ? qdisc_peek_dequeued+0x7b/0x350 [sch_qfq]
[  127.599670][  T453]  ? srso_alias_return_thunk+0x5/0xfbef5
[  127.599831][  T453]  tbf_dequeue+0x6b1/0x1098 [sch_tbf]
[  127.599988][  T453]  __qdisc_run+0x169/0x1900

The right thing to do in #1b is to grab the skb off gso_skb queue.
This patchset fixes that issue by changing #1b to use qdisc_dequeue_peeked()
method instead.

Fixes: e13e02a3c68d ("net_sched: SFB flow scheduler")
Signed-off-by: Victor Nogueria <victor@mojatatu.com>
---
 net/sched/sch_sfb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sched/sch_sfb.c b/net/sched/sch_sfb.c
index bd5ef561030f..d3ee8e5479b3 100644
--- a/net/sched/sch_sfb.c
+++ b/net/sched/sch_sfb.c
@@ -441,7 +441,7 @@ static struct sk_buff *sfb_dequeue(struct Qdisc *sch)
 	struct Qdisc *child = q->qdisc;
 	struct sk_buff *skb;
 
-	skb = child->dequeue(q->qdisc);
+	skb = qdisc_dequeue_peeked(child);
 
 	if (skb) {
 		qdisc_bstats_update(sch, skb);
-- 
2.34.1


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

* [PATCH net 3/3] selftests/tc-testing: Add tests that force red and sfb to dequeue from child's gso_skb
  2026-04-30 15:29 [PATCH net 0/3] Replace direct dequeue call with qdisc_dequeue_peeked Jamal Hadi Salim
  2026-04-30 15:29 ` [PATCH net 1/3] net/sched: sch_red: Replace direct dequeue call with peek and qdisc_dequeue_peeked Jamal Hadi Salim
  2026-04-30 15:29 ` [PATCH net 2/3] net/sched: sch_sfb: " Jamal Hadi Salim
@ 2026-04-30 15:29 ` Jamal Hadi Salim
  2026-05-02 18:20 ` [PATCH net 0/3] Replace direct dequeue call with qdisc_dequeue_peeked patchwork-bot+netdevbpf
  3 siblings, 0 replies; 7+ messages in thread
From: Jamal Hadi Salim @ 2026-04-30 15:29 UTC (permalink / raw)
  To: netdev
  Cc: Victor Nogueira, davem, kuba, edumazet, pabeni, horms, jiri,
	pctammela, ghandatmanas, rakshitawasthi17, security,
	Jamal Hadi Salim

From: Victor Nogueira <victor@mojatatu.com>

Create 4 test cases:
- Force red to dequeue from its child's gso_skb with qfq leaf
- Force sfb to dequeue from its child's gso_skb with qfq leaf
- Force red to dequeue from its child's gso_skb with dualpi2 leaf
- Force sfb to dequeue from its child's gso_skb with dualpi2 leaf

All of them have tbf followed by red (or sfb) followed by qfq (or
dualpi2). Since tbf calls its child's peek followed by
qdisc_dequeue_peeked, it will force red/sfb to call their child's peek.
In this case, since the child (qfq/dualpi2) has qdisc_peek_dequeued as
its peek callback, the packet will be stored in its gso_skb queue. During
the subsequent call to qdisc_dequeue_peeked, red/sfb will have to dequeue
from the child's gso_skb to retrieve the packet.
Not doing so will cause a NULL ptr deref which was happening before a
recent fix.

Acked-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
---
 .../tc-testing/tc-tests/infra/qdiscs.json     | 148 ++++++++++++++++++
 1 file changed, 148 insertions(+)

diff --git a/tools/testing/selftests/tc-testing/tc-tests/infra/qdiscs.json b/tools/testing/selftests/tc-testing/tc-tests/infra/qdiscs.json
index eefadd0546d3..b1f856cf62c1 100644
--- a/tools/testing/selftests/tc-testing/tc-tests/infra/qdiscs.json
+++ b/tools/testing/selftests/tc-testing/tc-tests/infra/qdiscs.json
@@ -1136,5 +1136,153 @@
         "teardown": [
             "$TC qdisc del dev $DUMMY handle 1: root"
         ]
+    },
+    {
+        "id": "7a5f",
+        "name": "Force red to dequeue from its child's gso_skb with qfq leaf",
+        "category": [
+            "qdisc",
+            "tbf",
+            "red",
+            "qfq"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$IP link set dev $DUMMY up || true",
+            "$IP addr add 10.10.11.10/24 dev $DUMMY || true",
+            "$TC qdisc add dev $DUMMY root handle 1: tbf rate 88bit burst 1661b peakrate 2257333 minburst 1024 limit 7b",
+            "$TC qdisc add dev $DUMMY parent 1: handle 2: red limit 757 min 16 max 24 avpkt 16",
+            "$TC qdisc add dev $DUMMY parent 2: handle 3: qfq",
+            "$TC class add dev $DUMMY classid 3:1 parent 3: qfq maxpkt 512 weight 1",
+            "$TC filter add dev $DUMMY parent 3: protocol ip prio 1 matchall classid 3:1 action ok"
+        ],
+        "cmdUnderTest": "ping -c 1 10.10.10.1 -W0.01 -I$DUMMY || true",
+        "expExitCode": "0",
+        "verifyCmd": "$TC -s -j qdisc ls dev $DUMMY parent 1:",
+        "matchJSON": [
+            {
+                "kind": "red",
+                "handle": "2:",
+                "bytes": 98,
+                "packets": 1,
+                "backlog": 0,
+                "qlen": 0
+            }
+        ],
+        "teardown": [
+            "$TC qdisc del dev $DUMMY handle 1: root"
+        ]
+    },
+    {
+        "id": "cdae",
+        "name": "Force sfb to dequeue from its child's gso_skb with qfq leaf",
+        "category": [
+            "qdisc",
+            "tbf",
+            "sfb",
+            "qfq"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$IP link set dev $DUMMY up || true",
+            "$IP addr add 10.10.11.10/24 dev $DUMMY || true",
+            "$TC qdisc add dev $DUMMY root handle 1: tbf rate 88bit burst 1661b peakrate 2257333 minburst 1024 limit 7b",
+            "$TC qdisc add dev $DUMMY parent 1: handle 2: sfb",
+            "$TC qdisc add dev $DUMMY parent 2: handle 3: qfq",
+            "$TC class add dev $DUMMY classid 3:1 parent 3: qfq maxpkt 512 weight 1",
+            "$TC filter add dev $DUMMY parent 3: protocol ip prio 1 matchall classid 3:1 action ok"
+        ],
+        "cmdUnderTest": "ping -c 1 10.10.10.1 -W0.01 -I$DUMMY || true",
+        "expExitCode": "0",
+        "verifyCmd": "$TC -s -j qdisc ls dev $DUMMY parent 1:",
+        "matchJSON": [
+            {
+                "kind": "sfb",
+                "handle": "2:",
+                "bytes": 98,
+                "packets": 1,
+                "backlog": 0,
+                "qlen": 0
+            }
+        ],
+        "teardown": [
+            "$TC qdisc del dev $DUMMY handle 1: root"
+        ]
+    },
+    {
+        "id": "291d",
+        "name": "Force red to dequeue from its child's gso_skb with dualpi2 leaf",
+        "category": [
+            "qdisc",
+            "tbf",
+            "red",
+            "dualpi2"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$IP link set dev $DUMMY up || true",
+            "$IP addr add 10.10.11.10/24 dev $DUMMY || true",
+            "$TC qdisc add dev $DUMMY root handle 1: tbf rate 88bit burst 1661b peakrate 2257333 minburst 1024 limit 7b",
+            "$TC qdisc add dev $DUMMY parent 1: handle 2: red limit 757 min 16 max 24 avpkt 16",
+            "$TC qdisc add dev $DUMMY parent 2: handle 3: dualpi2"
+        ],
+        "cmdUnderTest": "ping -c 1 10.10.10.1 -W0.01 -I$DUMMY || true",
+        "expExitCode": "0",
+        "verifyCmd": "$TC -s -j qdisc ls dev $DUMMY parent 1:",
+        "matchJSON": [
+            {
+                "kind": "red",
+                "handle": "2:",
+                "bytes": 98,
+                "packets": 1,
+                "backlog": 0,
+                "qlen": 0
+            }
+        ],
+        "teardown": [
+            "$TC qdisc del dev $DUMMY handle 1: root"
+        ]
+    },
+    {
+        "id": "9c6d",
+        "name": "Force sfb to dequeue from its child's gso_skb with dualpi2 leaf",
+        "category": [
+            "qdisc",
+            "tbf",
+            "sfb",
+            "dualpi2"
+        ],
+        "plugins": {
+            "requires": "nsPlugin"
+        },
+        "setup": [
+            "$IP link set dev $DUMMY up || true",
+            "$IP addr add 10.10.11.10/24 dev $DUMMY || true",
+            "$TC qdisc add dev $DUMMY root handle 1: tbf rate 88bit burst 1661b peakrate 2257333 minburst 1024 limit 7b",
+            "$TC qdisc add dev $DUMMY parent 1: handle 2: sfb",
+            "$TC qdisc add dev $DUMMY parent 2: handle 3: dualpi2"
+        ],
+        "cmdUnderTest": "ping -c 1 10.10.10.1 -W0.01 -I$DUMMY || true",
+        "expExitCode": "0",
+        "verifyCmd": "$TC -s -j qdisc ls dev $DUMMY parent 1:",
+        "matchJSON": [
+            {
+                "kind": "sfb",
+                "handle": "2:",
+                "bytes": 98,
+                "packets": 1,
+                "backlog": 0,
+                "qlen": 0
+            }
+        ],
+        "teardown": [
+            "$TC qdisc del dev $DUMMY handle 1: root"
+        ]
     }
 ]
-- 
2.34.1


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

* Re: [PATCH net 1/3] net/sched: sch_red: Replace direct dequeue call with peek and qdisc_dequeue_peeked
  2026-04-30 15:29 ` [PATCH net 1/3] net/sched: sch_red: Replace direct dequeue call with peek and qdisc_dequeue_peeked Jamal Hadi Salim
@ 2026-04-30 15:41   ` Eric Dumazet
  0 siblings, 0 replies; 7+ messages in thread
From: Eric Dumazet @ 2026-04-30 15:41 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: netdev, davem, kuba, pabeni, horms, jiri, victor, pctammela,
	ghandatmanas, rakshitawasthi17, security

On Thu, Apr 30, 2026 at 8:30 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> When red qdisc has children (eg qfq qdisc) whose peek() callback is
> qdisc_peek_dequeued(), we could get a kernel panic. When the parent of such
> qdiscs (eg illustrated in patch #3 as tbf) wants to retrieve an skb from
> its child (red in this case), it will do the following:
>  1a. do a peek() - and when sensing there's an skb the child can offer, then
>      - the child in this case(red) calls its child's (qfq) peek.
>         qfq does the right thing and will return the gso_skb queue packet.
>         Note: if there wasnt a gso_skb entry then qfq will store it there.
>  1b. invoke a dequeue() on the child (red). And herein lies the problem.
>      - red will call the child's dequeue() which will essentially just
>        try to grab something of qfq's queue.
>
>
> The right thing to do in #1b is to grab the skb off gso_skb queue.
> This patchset fixes that issue by changing #1b to use qdisc_dequeue_peeked()
> method instead.
>
>
> Fixes: 77be155cba4e ("pkt_sched: Add peek emulation for non-work-conserving qdiscs.")
> Reported-by: Manas <ghandatmanas@gmail.com>
> Reported-by: Rakshit Awasthi <rakshitawasthi17@gmail.com>
> Signed-off-by: Jamal Hadi Salim <jhs@mojatatu.com>

Reviewed-by: Eric Dumazet <edumazet@google.com>

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

* Re: [PATCH net 2/3] net/sched: sch_sfb: Replace direct dequeue call with peek and qdisc_dequeue_peeked
  2026-04-30 15:29 ` [PATCH net 2/3] net/sched: sch_sfb: " Jamal Hadi Salim
@ 2026-04-30 15:42   ` Eric Dumazet
  0 siblings, 0 replies; 7+ messages in thread
From: Eric Dumazet @ 2026-04-30 15:42 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: netdev, Victor Nogueria, davem, kuba, pabeni, horms, jiri,
	pctammela, ghandatmanas, rakshitawasthi17, security

On Thu, Apr 30, 2026 at 8:30 AM Jamal Hadi Salim <jhs@mojatatu.com> wrote:
>
> From: Victor Nogueria <victor@mojatatu.com>
>
> When sfb has children (eg qfq qdisc) whose peek() callback is
> qdisc_peek_dequeued(), we could get a kernel panic. When the parent of such
> qdiscs (eg illustrated in patch #3 as tbf) wants to retrieve an skb from
> its child (sfb in this case), it will do the following:
>  1a. do a peek() - and when sensing there's an skb the child can offer, then
>      - the child in this case(sfb) calls its child's (qfq) peek.
>         qfq does the right thing and will return the gso_skb queue packet.
>         Note: if there wasnt a gso_skb entry then qfq will store it there.
>  1b. invoke a dequeue() on the child (sfb). And herein lies the problem.
>      - sfb will call the child's dequeue() which will essentially just
>        try to grab something of qfq's queue.
>

> The right thing to do in #1b is to grab the skb off gso_skb queue.
> This patchset fixes that issue by changing #1b to use qdisc_dequeue_peeked()
> method instead.
>
> Fixes: e13e02a3c68d ("net_sched: SFB flow scheduler")
> Signed-off-by: Victor Nogueria <victor@mojatatu.com>

Reviewed-by: Eric Dumazet <edumazet@google.com>

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

* Re: [PATCH net 0/3] Replace direct dequeue call with qdisc_dequeue_peeked
  2026-04-30 15:29 [PATCH net 0/3] Replace direct dequeue call with qdisc_dequeue_peeked Jamal Hadi Salim
                   ` (2 preceding siblings ...)
  2026-04-30 15:29 ` [PATCH net 3/3] selftests/tc-testing: Add tests that force red and sfb to dequeue from child's gso_skb Jamal Hadi Salim
@ 2026-05-02 18:20 ` patchwork-bot+netdevbpf
  3 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-05-02 18:20 UTC (permalink / raw)
  To: Jamal Hadi Salim
  Cc: netdev, davem, kuba, edumazet, pabeni, horms, jiri, victor,
	pctammela, ghandatmanas, rakshitawasthi17, security

Hello:

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

On Thu, 30 Apr 2026 11:29:54 -0400 you wrote:
> When sfb and red qdiscs have children (eg qfq qdisc) whose peek() callback is
> qdisc_peek_dequeued(), we could get a kernel panic. When the parent of such
> qdiscs (eg illustrated in patch #3 as tbf) wants to retrieve an skb from
> its child (red/sfb in this case), it will do the following:
>  1a. do a peek() - and when sensing there's an skb the child can offer, then
>      - the child in this case(red/sfb) calls its child's (qfq) peek.
>         qfq does the right thing and will return the gso_skb queue packet.
>         Note: if there wasnt a gso_skb entry then qfq will store it there.
>  1b. invoke a dequeue() on the child (red/sfb). And herein lies the problem.
>      - red/sfb will call the child's dequeue() which will essentially just
>        try to grab something of qfq's queue.
> 
> [...]

Here is the summary with links:
  - [net,1/3] net/sched: sch_red: Replace direct dequeue call with peek and qdisc_dequeue_peeked
    https://git.kernel.org/netdev/net/c/458d5615272d
  - [net,2/3] net/sched: sch_sfb: Replace direct dequeue call with peek and qdisc_dequeue_peeked
    https://git.kernel.org/netdev/net/c/1b9bc71153b0
  - [net,3/3] selftests/tc-testing: Add tests that force red and sfb to dequeue from child's gso_skb
    https://git.kernel.org/netdev/net/c/3a3a30c14d7f

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-05-02 18:21 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-30 15:29 [PATCH net 0/3] Replace direct dequeue call with qdisc_dequeue_peeked Jamal Hadi Salim
2026-04-30 15:29 ` [PATCH net 1/3] net/sched: sch_red: Replace direct dequeue call with peek and qdisc_dequeue_peeked Jamal Hadi Salim
2026-04-30 15:41   ` Eric Dumazet
2026-04-30 15:29 ` [PATCH net 2/3] net/sched: sch_sfb: " Jamal Hadi Salim
2026-04-30 15:42   ` Eric Dumazet
2026-04-30 15:29 ` [PATCH net 3/3] selftests/tc-testing: Add tests that force red and sfb to dequeue from child's gso_skb Jamal Hadi Salim
2026-05-02 18:20 ` [PATCH net 0/3] Replace direct dequeue call with qdisc_dequeue_peeked 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