* [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