[PATCH 5.15.y 3/5] net_sched: sch_sfq: don't allow 1 packet limit

[Harshit Mogalapalli]

From: Octavian Purdila <ta...@google.com>

[ Upstream commit 10685681bafce6febb39770f3387621bf5d67d0b ]

The current implementation does not work correctly with a limit of
1. iproute2 actually checks for this and this patch adds the check in
kernel as well.

This fixes the following syzkaller reported crash:

UBSAN: array-index-out-of-bounds in net/sched/sch_sfq.c:210:6
index 65535 is out of range for type 'struct sfq_head[128]'
CPU: 0 PID: 2569 Comm: syz-executor101 Not tainted 5.10.0-smp-DEV #1
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 09/13/2024
Call Trace:
__dump_stack lib/dump_stack.c:79 [inline]
dump_stack+0x125/0x19f lib/dump_stack.c:120
ubsan_epilogue lib/ubsan.c:148 [inline]
__ubsan_handle_out_of_bounds+0xed/0x120 lib/ubsan.c:347
sfq_link net/sched/sch_sfq.c:210 [inline]
sfq_dec+0x528/0x600 net/sched/sch_sfq.c:238
sfq_dequeue+0x39b/0x9d0 net/sched/sch_sfq.c:500
sfq_reset+0x13/0x50 net/sched/sch_sfq.c:525
qdisc_reset+0xfe/0x510 net/sched/sch_generic.c:1026
tbf_reset+0x3d/0x100 net/sched/sch_tbf.c:319
qdisc_reset+0xfe/0x510 net/sched/sch_generic.c:1026
dev_reset_queue+0x8c/0x140 net/sched/sch_generic.c:1296
netdev_for_each_tx_queue include/linux/netdevice.h:2350 [inline]
dev_deactivate_many+0x6dc/0xc20 net/sched/sch_generic.c:1362
__dev_close_many+0x214/0x350 net/core/dev.c:1468
dev_close_many+0x207/0x510 net/core/dev.c:1506
unregister_netdevice_many+0x40f/0x16b0 net/core/dev.c:10738
unregister_netdevice_queue+0x2be/0x310 net/core/dev.c:10695
unregister_netdevice include/linux/netdevice.h:2893 [inline]
__tun_detach+0x6b6/0x1600 drivers/net/tun.c:689
tun_detach drivers/net/tun.c:705 [inline]
tun_chr_close+0x104/0x1b0 drivers/net/tun.c:3640
__fput+0x203/0x840 fs/file_table.c:280
task_work_run+0x129/0x1b0 kernel/task_work.c:185
exit_task_work include/linux/task_work.h:33 [inline]
do_exit+0x5ce/0x2200 kernel/exit.c:931
do_group_exit+0x144/0x310 kernel/exit.c:1046
__do_sys_exit_group kernel/exit.c:1057 [inline]
__se_sys_exit_group kernel/exit.c:1055 [inline]
__x64_sys_exit_group+0x3b/0x40 kernel/exit.c:1055
do_syscall_64+0x6c/0xd0
entry_SYSCALL_64_after_hwframe+0x61/0xcb
RIP: 0033:0x7fe5e7b52479
Code: Unable to access opcode bytes at RIP 0x7fe5e7b5244f.
RSP: 002b:00007ffd3c800398 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007fe5e7b52479
RDX: 000000000000003c RSI: 00000000000000e7 RDI: 0000000000000000
RBP: 00007fe5e7bcd2d0 R08: ffffffffffffffb8 R09: 0000000000000014
R10: 0000000000000000 R11: 0000000000000246 R12: 00007fe5e7bcd2d0
R13: 0000000000000000 R14: 00007fe5e7bcdd20 R15: 00007fe5e7b24270

The crash can be also be reproduced with the following (with a tc
recompiled to allow for sfq limits of 1):

tc qdisc add dev dummy0 handle 1: root tbf rate 1Kbit burst 100b lat 1s
../iproute2-6.9.0/tc/tc qdisc add dev dummy0 handle 2: parent 1:10 sfq limit 1
ifconfig dummy0 up
ping -I dummy0 -f -c2 -W0.1 8.8.8.8
sleep 1

Scenario that triggers the crash:

* the first packet is sent and queued in TBF and SFQ; qdisc qlen is 1

* TBF dequeues: it peeks from SFQ which moves the packet to the
gso_skb list and keeps qdisc qlen set to 1. TBF is out of tokens so
it schedules itself for later.

* the second packet is sent and TBF tries to queues it to SFQ. qdisc
qlen is now 2 and because the SFQ limit is 1 the packet is dropped
by SFQ. At this point qlen is 1, and all of the SFQ slots are empty,
however q->tail is not NULL.

At this point, assuming no more packets are queued, when sch_dequeue
runs again it will decrement the qlen for the current empty slot
causing an underflow and the subsequent out of bounds access.

Reported-by: syzbot <syzk...@googlegroups.com>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Octavian Purdila <ta...@google.com>
Reviewed-by: Eric Dumazet <edum...@google.com>
Link: https://patch.msgid.link/20241204030520....@google.com
Signed-off-by: Jakub Kicinski <ku...@kernel.org>
(cherry picked from commit 10685681bafce6febb39770f3387621bf5d67d0b)
Signed-off-by: Harshit Mogalapalli <harshit.m....@oracle.com>
---
net/sched/sch_sfq.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 714bdc2c5a68..505209a932ab 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -649,6 +649,10 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt,
if (!p)
return -ENOMEM;
}
+ if (ctl->limit == 1) {
+ NL_SET_ERR_MSG_MOD(extack, "invalid limit");
+ return -EINVAL;
+ }
sch_tree_lock(sch);
if (ctl->quantum)
q->quantum = ctl->quantum;
--
2.47.1

[Harshit Mogalapalli]

index 2fb2dcd0ae2a..16dd1d802e64 100644

[Eric Dumazet]

From: Octavian Purdila <ta...@google.com>

commit 10685681bafce6febb39770f3387621bf5d67d0b upstream.

---
net/sched/sch_sfq.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c

index 714bdc2c5a682a9767ce5e8a404aa7b4889604a6..505209a932ab73a91a0b15f990d4c9d7a206a05a 100644

--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -649,6 +649,10 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt,
if (!p)
return -ENOMEM;
}
+ if (ctl->limit == 1) {
+ NL_SET_ERR_MSG_MOD(extack, "invalid limit");
+ return -EINVAL;
+ }
sch_tree_lock(sch);
if (ctl->quantum)
q->quantum = ctl->quantum;
--

2.50.0.rc2.701.gf1e915cc24-goog

[gre...@linuxfoundation.org]

This is a note to let you know that I've just added the patch titled

net_sched: sch_sfq: don't allow 1 packet limit

to the 5.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
net_sched-sch_sfq-don-t-allow-1-packet-limit.patch
and it can be found in the queue-5.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <sta...@vger.kernel.org> know about it.


From stable+bounces-152671-greg=kroa...@vger.kernel.org Sun Jun 15 19:52:16 2025
From: Harshit Mogalapalli <harshit.m....@oracle.com>
Date: Sun, 15 Jun 2025 10:51:51 -0700
Subject: net_sched: sch_sfq: don't allow 1 packet limit
To: sta...@vger.kernel.org
Cc: ta...@google.com, edum...@google.com, syzbot <syzk...@googlegroups.com>, Jakub Kicinski <ku...@kernel.org>, Harshit Mogalapalli <harshit.m....@oracle.com>
Message-ID: <20250615175153.1610731-...@oracle.com>

Signed-off-by: Harshit Mogalapalli <harshit.m....@oracle.com>
Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>

---
net/sched/sch_sfq.c | 4 ++++
1 file changed, 4 insertions(+)

--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -652,6 +652,10 @@ static int sfq_change(struct Qdisc *sch,

if (!p)
return -ENOMEM;
}
+ if (ctl->limit == 1) {
+ NL_SET_ERR_MSG_MOD(extack, "invalid limit");
+ return -EINVAL;
+ }
sch_tree_lock(sch);
if (ctl->quantum)
q->quantum = ctl->quantum;

Patches currently in stable-queue which might be from harshit.m....@oracle.com are

queue-5.10/net_sched-sch_sfq-annotate-data-races-around-q-perturb_period.patch
queue-5.10/net_sched-sch_sfq-use-a-temporary-work-area-for-validating-configuration.patch
queue-5.10/net_sched-sch_sfq-move-the-limit-validation.patch
queue-5.10/net_sched-sch_sfq-don-t-allow-1-packet-limit.patch
queue-5.10/net_sched-sch_sfq-handle-bigger-packets.patch

[gre...@linuxfoundation.org]

This is a note to let you know that I've just added the patch titled

net_sched: sch_sfq: don't allow 1 packet limit

to the 5.15-stable tree which can be found at:

http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
net_sched-sch_sfq-don-t-allow-1-packet-limit.patch

and it can be found in the queue-5.15 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <sta...@vger.kernel.org> know about it.

From stable+bounces-155164-greg=kroa...@vger.kernel.org Fri Jun 20 17:50:57 2025
From: Eric Dumazet <edum...@google.com>
Date: Fri, 20 Jun 2025 15:46:19 +0000

Subject: net_sched: sch_sfq: don't allow 1 packet limit
To: sta...@vger.kernel.org

Cc: Octavian Purdila <ta...@google.com>, syzbot <syzk...@googlegroups.com>, Eric Dumazet <edum...@google.com>, Jakub Kicinski <ku...@kernel.org>
Message-ID: <20250620154623.3...@google.com>

Signed-off-by: Greg Kroah-Hartman <gre...@linuxfoundation.org>

---
net/sched/sch_sfq.c | 4 ++++
1 file changed, 4 insertions(+)

--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -652,6 +652,10 @@ static int sfq_change(struct Qdisc *sch,

if (!p)
return -ENOMEM;
}
+ if (ctl->limit == 1) {
+ NL_SET_ERR_MSG_MOD(extack, "invalid limit");
+ return -EINVAL;
+ }
sch_tree_lock(sch);
if (ctl->quantum)
q->quantum = ctl->quantum;

Patches currently in stable-queue which might be from edum...@google.com are

queue-5.15/net_sched-tbf-fix-a-race-in-tbf_change.patch
queue-5.15/net-atm-fix-proc-net-atm-lec-handling.patch
queue-5.15/net_sched-red-fix-a-race-in-__red_change.patch
queue-5.15/tcp-fix-tcp_packet_delayed-for-tcp_is_non_sack_preve.patch
queue-5.15/net_sched-sch_sfq-annotate-data-races-around-q-perturb_period.patch
queue-5.15/net_sched-ets-fix-a-race-in-ets_qdisc_change.patch
queue-5.15/calipso-unlock-rcu-before-returning-eafnosupport.patch
queue-5.15/net_sched-sch_sfq-use-a-temporary-work-area-for-validating-configuration.patch
queue-5.15/tcp-fix-initial-tp-rcvq_space.space-value-for-passiv.patch
queue-5.15/net_sched-prio-fix-a-race-in-prio_tune.patch
queue-5.15/net-atm-add-lec_mutex.patch
queue-5.15/net_sched-sch_sfq-fix-a-potential-crash-on-gso_skb-h.patch
queue-5.15/net_sched-sch_sfq-move-the-limit-validation.patch
queue-5.15/net_sched-sch_sfq-don-t-allow-1-packet-limit.patch
queue-5.15/net_sched-sch_sfq-handle-bigger-packets.patch
queue-5.15/ib-cm-use-rwlock-for-mad-agent-lock.patch
queue-5.15/tcp-always-seek-for-minimal-rtt-in-tcp_rcv_rtt_updat.patch
queue-5.15/net_sched-sch_sfq-reject-invalid-perturb-period.patch