* [PATCH 5.4.y 0/6] Backport few sch_sfq fixes
@ 2025-07-17 12:45 Harshit Mogalapalli
2025-07-17 12:45 ` [PATCH 5.4.y 1/6] net_sched: sch_sfq: annotate data-races around q->perturb_period Harshit Mogalapalli
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Harshit Mogalapalli @ 2025-07-17 12:45 UTC (permalink / raw)
To: stable; +Cc: edumazet, tavip, Harshit Mogalapalli
commit: 10685681bafc ("net_sched: sch_sfq: don't allow 1 packet limit")
fixes CVE-2024-57996 and commit: b3bf8f63e617 ("net_sched: sch_sfq: move
the limit validation") fixes CVE-2025-37752 and commit: 7ca52541c05c
("net_sched: sch_sfq: reject invalid perturb period") fixes
CVE-2025-38193.
Patches 3, 5, 6 are CVE fixes for above mentioned CVEs. Patch 1,2 and 4
are pulled in as stable-deps.
Testing performed on the patched 5.4.295 kernel with the above 5
patches: (Used latest upstream kselftests for tc-testing)
$ uname -a
Linux hamogala-kdevoci8-1 5.4.295-master.20250717.el8.rc2.x86_64 #1 SMP Thu Jul 17 00:57:21 PDT 2025 x86_64 x86_64 x86_64 GNU/Linux
$ python3.12 ./tdc.py -f tc-tests/qdiscs/sfq.json
-- ns/SubPlugin.__init__
Test 7482: Create SFQ with default setting
Test c186: Create SFQ with limit setting
Test ae23: Create SFQ with perturb setting
Test a430: Create SFQ with quantum setting
Test 4539: Create SFQ with divisor setting
Test b089: Create SFQ with flows setting
Test 99a0: Create SFQ with depth setting
Test 7389: Create SFQ with headdrop setting
Test 6472: Create SFQ with redflowlimit setting
Test 8929: Show SFQ class
Test 4d6f: Check that limit of 1 is rejected
Test 7f8f: Check that a derived limit of 1 is rejected (limit 2 depth 1 flows 1)
Test 5168: Check that a derived limit of 1 is rejected (limit 2 depth 1 divisor 1)
All test results:
1..13
ok 1 7482 - Create SFQ with default setting
ok 2 c186 - Create SFQ with limit setting
ok 3 ae23 - Create SFQ with perturb setting
ok 4 a430 - Create SFQ with quantum setting
ok 5 4539 - Create SFQ with divisor setting
ok 6 b089 - Create SFQ with flows setting
ok 7 99a0 - Create SFQ with depth setting
ok 8 7389 - Create SFQ with headdrop setting
ok 9 6472 - Create SFQ with redflowlimit setting
ok 10 8929 - Show SFQ class
ok 11 4d6f - Check that limit of 1 is rejected
ok 12 7f8f - Check that a derived limit of 1 is rejected (limit 2 depth 1 flows 1)
ok 13 5168 - Check that a derived limit of 1 is rejected (limit 2 depth 1 divisor 1)
Thanks,
Harshit
Eric Dumazet (3):
net_sched: sch_sfq: annotate data-races around q->perturb_period
net_sched: sch_sfq: handle bigger packets
net_sched: sch_sfq: reject invalid perturb period
Octavian Purdila (3):
net_sched: sch_sfq: don't allow 1 packet limit
net_sched: sch_sfq: use a temporary work area for validating
configuration
net_sched: sch_sfq: move the limit validation
net/sched/sch_sfq.c | 114 +++++++++++++++++++++++++++++---------------
1 file changed, 75 insertions(+), 39 deletions(-)
--
2.47.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 5.4.y 1/6] net_sched: sch_sfq: annotate data-races around q->perturb_period
2025-07-17 12:45 [PATCH 5.4.y 0/6] Backport few sch_sfq fixes Harshit Mogalapalli
@ 2025-07-17 12:45 ` Harshit Mogalapalli
2025-07-18 1:34 ` Sasha Levin
2025-07-17 12:45 ` [PATCH 5.4.y 2/6] net_sched: sch_sfq: handle bigger packets Harshit Mogalapalli
` (4 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Harshit Mogalapalli @ 2025-07-17 12:45 UTC (permalink / raw)
To: stable; +Cc: edumazet, tavip, Simon Horman, Jakub Kicinski,
Harshit Mogalapalli
From: Eric Dumazet <edumazet@google.com>
[ Upstream commit a17ef9e6c2c1cf0fc6cd6ca6a9ce525c67d1da7f ]
sfq_perturbation() reads q->perturb_period locklessly.
Add annotations to fix potential issues.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://lore.kernel.org/r/20240430180015.3111398-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
(cherry picked from commit a17ef9e6c2c1cf0fc6cd6ca6a9ce525c67d1da7f)
[Harshit: Backport to 5.4.y, conflicts resolved due to missing commit:
d636fc5dd692 ("net: sched: add rcu annotations around
qdisc->qdisc_sleeping")in 5.4.y]
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
net/sched/sch_sfq.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index acda65371028..7ca33cfbd03b 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -611,6 +611,7 @@ static void sfq_perturbation(struct timer_list *t)
struct Qdisc *sch = q->sch;
spinlock_t *root_lock = qdisc_lock(qdisc_root_sleeping(sch));
siphash_key_t nkey;
+ int period;
get_random_bytes(&nkey, sizeof(nkey));
spin_lock(root_lock);
@@ -619,8 +620,12 @@ static void sfq_perturbation(struct timer_list *t)
sfq_rehash(sch);
spin_unlock(root_lock);
- if (q->perturb_period)
- mod_timer(&q->perturb_timer, jiffies + q->perturb_period);
+ /* q->perturb_period can change under us from
+ * sfq_change() and sfq_destroy().
+ */
+ period = READ_ONCE(q->perturb_period);
+ if (period)
+ mod_timer(&q->perturb_timer, jiffies + period);
}
static int sfq_change(struct Qdisc *sch, struct nlattr *opt)
@@ -662,7 +667,7 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt)
q->quantum = ctl->quantum;
q->scaled_quantum = SFQ_ALLOT_SIZE(q->quantum);
}
- q->perturb_period = ctl->perturb_period * HZ;
+ WRITE_ONCE(q->perturb_period, ctl->perturb_period * HZ);
if (ctl->flows)
q->maxflows = min_t(u32, ctl->flows, SFQ_MAX_FLOWS);
if (ctl->divisor) {
@@ -724,7 +729,7 @@ static void sfq_destroy(struct Qdisc *sch)
struct sfq_sched_data *q = qdisc_priv(sch);
tcf_block_put(q->block);
- q->perturb_period = 0;
+ WRITE_ONCE(q->perturb_period, 0);
del_timer_sync(&q->perturb_timer);
sfq_free(q->ht);
sfq_free(q->slots);
--
2.47.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5.4.y 2/6] net_sched: sch_sfq: handle bigger packets
2025-07-17 12:45 [PATCH 5.4.y 0/6] Backport few sch_sfq fixes Harshit Mogalapalli
2025-07-17 12:45 ` [PATCH 5.4.y 1/6] net_sched: sch_sfq: annotate data-races around q->perturb_period Harshit Mogalapalli
@ 2025-07-17 12:45 ` Harshit Mogalapalli
2025-07-18 1:34 ` Sasha Levin
2025-07-17 12:45 ` [PATCH 5.4.y 3/6] net_sched: sch_sfq: don't allow 1 packet limit Harshit Mogalapalli
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Harshit Mogalapalli @ 2025-07-17 12:45 UTC (permalink / raw)
To: stable
Cc: edumazet, tavip, Toke Høiland-Jørgensen, Jakub Kicinski,
Harshit Mogalapalli
From: Eric Dumazet <edumazet@google.com>
[ Upstream commit e4650d7ae4252f67e997a632adfae0dd74d3a99a ]
SFQ has an assumption on dealing with packets smaller than 64KB.
Even before BIG TCP, TCA_STAB can provide arbitrary big values
in qdisc_pkt_len(skb)
It is time to switch (struct sfq_slot)->allot to a 32bit field.
sizeof(struct sfq_slot) is now 64 bytes, giving better cache locality.
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Toke Høiland-Jørgensen <toke@redhat.com>
Link: https://patch.msgid.link/20241008111603.653140-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
(cherry picked from commit e4650d7ae4252f67e997a632adfae0dd74d3a99a)
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
net/sched/sch_sfq.c | 39 +++++++++++++--------------------------
1 file changed, 13 insertions(+), 26 deletions(-)
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 7ca33cfbd03b..69e7d01c8518 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -77,12 +77,6 @@
#define SFQ_EMPTY_SLOT 0xffff
#define SFQ_DEFAULT_HASH_DIVISOR 1024
-/* We use 16 bits to store allot, and want to handle packets up to 64K
- * Scale allot by 8 (1<<3) so that no overflow occurs.
- */
-#define SFQ_ALLOT_SHIFT 3
-#define SFQ_ALLOT_SIZE(X) DIV_ROUND_UP(X, 1 << SFQ_ALLOT_SHIFT)
-
/* This type should contain at least SFQ_MAX_DEPTH + 1 + SFQ_MAX_FLOWS values */
typedef u16 sfq_index;
@@ -104,7 +98,7 @@ struct sfq_slot {
sfq_index next; /* next slot in sfq RR chain */
struct sfq_head dep; /* anchor in dep[] chains */
unsigned short hash; /* hash value (index in ht[]) */
- short allot; /* credit for this slot */
+ int allot; /* credit for this slot */
unsigned int backlog;
struct red_vars vars;
@@ -120,7 +114,6 @@ struct sfq_sched_data {
siphash_key_t perturbation;
u8 cur_depth; /* depth of longest slot */
u8 flags;
- unsigned short scaled_quantum; /* SFQ_ALLOT_SIZE(quantum) */
struct tcf_proto __rcu *filter_list;
struct tcf_block *block;
sfq_index *ht; /* Hash table ('divisor' slots) */
@@ -459,7 +452,7 @@ sfq_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct sk_buff **to_free)
*/
q->tail = slot;
/* We could use a bigger initial quantum for new flows */
- slot->allot = q->scaled_quantum;
+ slot->allot = q->quantum;
}
if (++sch->q.qlen <= q->limit)
return NET_XMIT_SUCCESS;
@@ -496,7 +489,7 @@ sfq_dequeue(struct Qdisc *sch)
slot = &q->slots[a];
if (slot->allot <= 0) {
q->tail = slot;
- slot->allot += q->scaled_quantum;
+ slot->allot += q->quantum;
goto next_slot;
}
skb = slot_dequeue_head(slot);
@@ -515,7 +508,7 @@ sfq_dequeue(struct Qdisc *sch)
}
q->tail->next = next_a;
} else {
- slot->allot -= SFQ_ALLOT_SIZE(qdisc_pkt_len(skb));
+ slot->allot -= qdisc_pkt_len(skb);
}
return skb;
}
@@ -598,7 +591,7 @@ static void sfq_rehash(struct Qdisc *sch)
q->tail->next = x;
}
q->tail = slot;
- slot->allot = q->scaled_quantum;
+ slot->allot = q->quantum;
}
}
sch->q.qlen -= dropped;
@@ -628,7 +621,8 @@ static void sfq_perturbation(struct timer_list *t)
mod_timer(&q->perturb_timer, jiffies + period);
}
-static int sfq_change(struct Qdisc *sch, struct nlattr *opt)
+static int sfq_change(struct Qdisc *sch, struct nlattr *opt,
+ struct netlink_ext_ack *extack)
{
struct sfq_sched_data *q = qdisc_priv(sch);
struct tc_sfq_qopt *ctl = nla_data(opt);
@@ -646,14 +640,10 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt)
(!is_power_of_2(ctl->divisor) || ctl->divisor > 65536))
return -EINVAL;
- /* slot->allot is a short, make sure quantum is not too big. */
- if (ctl->quantum) {
- unsigned int scaled = SFQ_ALLOT_SIZE(ctl->quantum);
-
- if (scaled <= 0 || scaled > SHRT_MAX)
- return -EINVAL;
+ if ((int)ctl->quantum < 0) {
+ NL_SET_ERR_MSG_MOD(extack, "invalid quantum");
+ return -EINVAL;
}
-
if (ctl_v1 && !red_check_params(ctl_v1->qth_min, ctl_v1->qth_max,
ctl_v1->Wlog, ctl_v1->Scell_log, NULL))
return -EINVAL;
@@ -663,10 +653,8 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt)
return -ENOMEM;
}
sch_tree_lock(sch);
- if (ctl->quantum) {
+ if (ctl->quantum)
q->quantum = ctl->quantum;
- q->scaled_quantum = SFQ_ALLOT_SIZE(q->quantum);
- }
WRITE_ONCE(q->perturb_period, ctl->perturb_period * HZ);
if (ctl->flows)
q->maxflows = min_t(u32, ctl->flows, SFQ_MAX_FLOWS);
@@ -762,12 +750,11 @@ static int sfq_init(struct Qdisc *sch, struct nlattr *opt,
q->divisor = SFQ_DEFAULT_HASH_DIVISOR;
q->maxflows = SFQ_DEFAULT_FLOWS;
q->quantum = psched_mtu(qdisc_dev(sch));
- q->scaled_quantum = SFQ_ALLOT_SIZE(q->quantum);
q->perturb_period = 0;
get_random_bytes(&q->perturbation, sizeof(q->perturbation));
if (opt) {
- int err = sfq_change(sch, opt);
+ int err = sfq_change(sch, opt, extack);
if (err)
return err;
}
@@ -878,7 +865,7 @@ static int sfq_dump_class_stats(struct Qdisc *sch, unsigned long cl,
if (idx != SFQ_EMPTY_SLOT) {
const struct sfq_slot *slot = &q->slots[idx];
- xstats.allot = slot->allot << SFQ_ALLOT_SHIFT;
+ xstats.allot = slot->allot;
qs.qlen = slot->qlen;
qs.backlog = slot->backlog;
}
--
2.47.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5.4.y 3/6] net_sched: sch_sfq: don't allow 1 packet limit
2025-07-17 12:45 [PATCH 5.4.y 0/6] Backport few sch_sfq fixes Harshit Mogalapalli
2025-07-17 12:45 ` [PATCH 5.4.y 1/6] net_sched: sch_sfq: annotate data-races around q->perturb_period Harshit Mogalapalli
2025-07-17 12:45 ` [PATCH 5.4.y 2/6] net_sched: sch_sfq: handle bigger packets Harshit Mogalapalli
@ 2025-07-17 12:45 ` Harshit Mogalapalli
2025-07-18 1:34 ` Sasha Levin
2025-07-17 12:45 ` [PATCH 5.4.y 4/6] net_sched: sch_sfq: use a temporary work area for validating configuration Harshit Mogalapalli
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Harshit Mogalapalli @ 2025-07-17 12:45 UTC (permalink / raw)
To: stable; +Cc: edumazet, tavip, syzbot, Jakub Kicinski, Harshit Mogalapalli
From: Octavian Purdila <tavip@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 <syzkaller@googlegroups.com>
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Octavian Purdila <tavip@google.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Link: https://patch.msgid.link/20241204030520.2084663-2-tavip@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
(cherry picked from commit 10685681bafce6febb39770f3387621bf5d67d0b)
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@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 69e7d01c8518..aac1a4783f11 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -652,6 +652,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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5.4.y 4/6] net_sched: sch_sfq: use a temporary work area for validating configuration
2025-07-17 12:45 [PATCH 5.4.y 0/6] Backport few sch_sfq fixes Harshit Mogalapalli
` (2 preceding siblings ...)
2025-07-17 12:45 ` [PATCH 5.4.y 3/6] net_sched: sch_sfq: don't allow 1 packet limit Harshit Mogalapalli
@ 2025-07-17 12:45 ` Harshit Mogalapalli
2025-07-18 1:34 ` Sasha Levin
2025-07-17 12:45 ` [PATCH 5.4.y 5/6] net_sched: sch_sfq: move the limit validation Harshit Mogalapalli
2025-07-17 12:45 ` [PATCH 5.4.y 6/6] net_sched: sch_sfq: reject invalid perturb period Harshit Mogalapalli
5 siblings, 1 reply; 13+ messages in thread
From: Harshit Mogalapalli @ 2025-07-17 12:45 UTC (permalink / raw)
To: stable; +Cc: edumazet, tavip, Cong Wang, David S. Miller, Harshit Mogalapalli
From: Octavian Purdila <tavip@google.com>
[ Upstream commit 8c0cea59d40cf6dd13c2950437631dd614fbade6 ]
Many configuration parameters have influence on others (e.g. divisor
-> flows -> limit, depth -> limit) and so it is difficult to correctly
do all of the validation before applying the configuration. And if a
validation error is detected late it is difficult to roll back a
partially applied configuration.
To avoid these issues use a temporary work area to update and validate
the configuration and only then apply the configuration to the
internal state.
Signed-off-by: Octavian Purdila <tavip@google.com>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit 8c0cea59d40cf6dd13c2950437631dd614fbade6)
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
net/sched/sch_sfq.c | 56 +++++++++++++++++++++++++++++++++++----------
1 file changed, 44 insertions(+), 12 deletions(-)
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index aac1a4783f11..d49edcb9729e 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -631,6 +631,15 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt,
struct red_parms *p = NULL;
struct sk_buff *to_free = NULL;
struct sk_buff *tail = NULL;
+ unsigned int maxflows;
+ unsigned int quantum;
+ unsigned int divisor;
+ int perturb_period;
+ u8 headdrop;
+ u8 maxdepth;
+ int limit;
+ u8 flags;
+
if (opt->nla_len < nla_attr_size(sizeof(*ctl)))
return -EINVAL;
@@ -656,36 +665,59 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt,
NL_SET_ERR_MSG_MOD(extack, "invalid limit");
return -EINVAL;
}
+
sch_tree_lock(sch);
+
+ limit = q->limit;
+ divisor = q->divisor;
+ headdrop = q->headdrop;
+ maxdepth = q->maxdepth;
+ maxflows = q->maxflows;
+ perturb_period = q->perturb_period;
+ quantum = q->quantum;
+ flags = q->flags;
+
+ /* update and validate configuration */
if (ctl->quantum)
- q->quantum = ctl->quantum;
- WRITE_ONCE(q->perturb_period, ctl->perturb_period * HZ);
+ quantum = ctl->quantum;
+ perturb_period = ctl->perturb_period * HZ;
if (ctl->flows)
- q->maxflows = min_t(u32, ctl->flows, SFQ_MAX_FLOWS);
+ maxflows = min_t(u32, ctl->flows, SFQ_MAX_FLOWS);
if (ctl->divisor) {
- q->divisor = ctl->divisor;
- q->maxflows = min_t(u32, q->maxflows, q->divisor);
+ divisor = ctl->divisor;
+ maxflows = min_t(u32, maxflows, divisor);
}
if (ctl_v1) {
if (ctl_v1->depth)
- q->maxdepth = min_t(u32, ctl_v1->depth, SFQ_MAX_DEPTH);
+ maxdepth = min_t(u32, ctl_v1->depth, SFQ_MAX_DEPTH);
if (p) {
- swap(q->red_parms, p);
- red_set_parms(q->red_parms,
+ red_set_parms(p,
ctl_v1->qth_min, ctl_v1->qth_max,
ctl_v1->Wlog,
ctl_v1->Plog, ctl_v1->Scell_log,
NULL,
ctl_v1->max_P);
}
- q->flags = ctl_v1->flags;
- q->headdrop = ctl_v1->headdrop;
+ flags = ctl_v1->flags;
+ headdrop = ctl_v1->headdrop;
}
if (ctl->limit) {
- q->limit = min_t(u32, ctl->limit, q->maxdepth * q->maxflows);
- q->maxflows = min_t(u32, q->maxflows, q->limit);
+ limit = min_t(u32, ctl->limit, maxdepth * maxflows);
+ maxflows = min_t(u32, maxflows, limit);
}
+ /* commit configuration */
+ q->limit = limit;
+ q->divisor = divisor;
+ q->headdrop = headdrop;
+ q->maxdepth = maxdepth;
+ q->maxflows = maxflows;
+ WRITE_ONCE(q->perturb_period, perturb_period);
+ q->quantum = quantum;
+ q->flags = flags;
+ if (p)
+ swap(q->red_parms, p);
+
qlen = sch->q.qlen;
while (sch->q.qlen > q->limit) {
dropped += sfq_drop(sch, &to_free);
--
2.47.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5.4.y 5/6] net_sched: sch_sfq: move the limit validation
2025-07-17 12:45 [PATCH 5.4.y 0/6] Backport few sch_sfq fixes Harshit Mogalapalli
` (3 preceding siblings ...)
2025-07-17 12:45 ` [PATCH 5.4.y 4/6] net_sched: sch_sfq: use a temporary work area for validating configuration Harshit Mogalapalli
@ 2025-07-17 12:45 ` Harshit Mogalapalli
2025-07-18 1:34 ` Sasha Levin
2025-07-17 12:45 ` [PATCH 5.4.y 6/6] net_sched: sch_sfq: reject invalid perturb period Harshit Mogalapalli
5 siblings, 1 reply; 13+ messages in thread
From: Harshit Mogalapalli @ 2025-07-17 12:45 UTC (permalink / raw)
To: stable
Cc: edumazet, tavip, syzbot, Cong Wang, David S. Miller,
Harshit Mogalapalli
From: Octavian Purdila <tavip@google.com>
[ Upstream commit b3bf8f63e6179076b57c9de660c9f80b5abefe70 ]
It is not sufficient to directly validate the limit on the data that
the user passes as it can be updated based on how the other parameters
are changed.
Move the check at the end of the configuration update process to also
catch scenarios where the limit is indirectly updated, for example
with the following configurations:
tc qdisc add dev dummy0 handle 1: root sfq limit 2 flows 1 depth 1
tc qdisc add dev dummy0 handle 1: root sfq limit 2 flows 1 divisor 1
This fixes the following syzkaller reported crash:
------------[ cut here ]------------
UBSAN: array-index-out-of-bounds in net/sched/sch_sfq.c:203:6
index 65535 is out of range for type 'struct sfq_head[128]'
CPU: 1 UID: 0 PID: 3037 Comm: syz.2.16 Not tainted 6.14.0-rc2-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 12/27/2024
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:94 [inline]
dump_stack_lvl+0x201/0x300 lib/dump_stack.c:120
ubsan_epilogue lib/ubsan.c:231 [inline]
__ubsan_handle_out_of_bounds+0xf5/0x120 lib/ubsan.c:429
sfq_link net/sched/sch_sfq.c:203 [inline]
sfq_dec+0x53c/0x610 net/sched/sch_sfq.c:231
sfq_dequeue+0x34e/0x8c0 net/sched/sch_sfq.c:493
sfq_reset+0x17/0x60 net/sched/sch_sfq.c:518
qdisc_reset+0x12e/0x600 net/sched/sch_generic.c:1035
tbf_reset+0x41/0x110 net/sched/sch_tbf.c:339
qdisc_reset+0x12e/0x600 net/sched/sch_generic.c:1035
dev_reset_queue+0x100/0x1b0 net/sched/sch_generic.c:1311
netdev_for_each_tx_queue include/linux/netdevice.h:2590 [inline]
dev_deactivate_many+0x7e5/0xe70 net/sched/sch_generic.c:1375
Reported-by: syzbot <syzkaller@googlegroups.com>
Fixes: 10685681bafc ("net_sched: sch_sfq: don't allow 1 packet limit")
Signed-off-by: Octavian Purdila <tavip@google.com>
Acked-by: Cong Wang <xiyou.wangcong@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
(cherry picked from commit b3bf8f63e6179076b57c9de660c9f80b5abefe70)
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
net/sched/sch_sfq.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index d49edcb9729e..9e8601e64508 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -661,10 +661,6 @@ 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);
@@ -705,6 +701,12 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt,
limit = min_t(u32, ctl->limit, maxdepth * maxflows);
maxflows = min_t(u32, maxflows, limit);
}
+ if (limit == 1) {
+ sch_tree_unlock(sch);
+ kfree(p);
+ NL_SET_ERR_MSG_MOD(extack, "invalid limit");
+ return -EINVAL;
+ }
/* commit configuration */
q->limit = limit;
--
2.47.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5.4.y 6/6] net_sched: sch_sfq: reject invalid perturb period
2025-07-17 12:45 [PATCH 5.4.y 0/6] Backport few sch_sfq fixes Harshit Mogalapalli
` (4 preceding siblings ...)
2025-07-17 12:45 ` [PATCH 5.4.y 5/6] net_sched: sch_sfq: move the limit validation Harshit Mogalapalli
@ 2025-07-17 12:45 ` Harshit Mogalapalli
2025-07-18 1:34 ` Sasha Levin
5 siblings, 1 reply; 13+ messages in thread
From: Harshit Mogalapalli @ 2025-07-17 12:45 UTC (permalink / raw)
To: stable; +Cc: edumazet, tavip, Gerrard Tai, Jakub Kicinski, Harshit Mogalapalli
From: Eric Dumazet <edumazet@google.com>
[ Upstream commit 7ca52541c05c832d32b112274f81a985101f9ba8 ]
Gerrard Tai reported that SFQ perturb_period has no range check yet,
and this can be used to trigger a race condition fixed in a separate patch.
We want to make sure ctl->perturb_period * HZ will not overflow
and is positive.
Tested:
tc qd add dev lo root sfq perturb -10 # negative value : error
Error: sch_sfq: invalid perturb period.
tc qd add dev lo root sfq perturb 1000000000 # too big : error
Error: sch_sfq: invalid perturb period.
tc qd add dev lo root sfq perturb 2000000 # acceptable value
tc -s -d qd sh dev lo
qdisc sfq 8005: root refcnt 2 limit 127p quantum 64Kb depth 127 flows 128 divisor 1024 perturb 2000000sec
Sent 0 bytes 0 pkt (dropped 0, overlimits 0 requeues 0)
backlog 0b 0p requeues 0
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Reported-by: Gerrard Tai <gerrard.tai@starlabs.sg>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: stable@vger.kernel.org
Link: https://patch.msgid.link/20250611083501.1810459-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
(cherry picked from commit 7ca52541c05c832d32b112274f81a985101f9ba8)
Signed-off-by: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
---
net/sched/sch_sfq.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/net/sched/sch_sfq.c b/net/sched/sch_sfq.c
index 9e8601e64508..eaaa5d0e17a7 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -653,6 +653,14 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt,
NL_SET_ERR_MSG_MOD(extack, "invalid quantum");
return -EINVAL;
}
+
+ if (ctl->perturb_period < 0 ||
+ ctl->perturb_period > INT_MAX / HZ) {
+ NL_SET_ERR_MSG_MOD(extack, "invalid perturb period");
+ return -EINVAL;
+ }
+ perturb_period = ctl->perturb_period * HZ;
+
if (ctl_v1 && !red_check_params(ctl_v1->qth_min, ctl_v1->qth_max,
ctl_v1->Wlog, ctl_v1->Scell_log, NULL))
return -EINVAL;
@@ -669,14 +677,12 @@ static int sfq_change(struct Qdisc *sch, struct nlattr *opt,
headdrop = q->headdrop;
maxdepth = q->maxdepth;
maxflows = q->maxflows;
- perturb_period = q->perturb_period;
quantum = q->quantum;
flags = q->flags;
/* update and validate configuration */
if (ctl->quantum)
quantum = ctl->quantum;
- perturb_period = ctl->perturb_period * HZ;
if (ctl->flows)
maxflows = min_t(u32, ctl->flows, SFQ_MAX_FLOWS);
if (ctl->divisor) {
--
2.47.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 5.4.y 1/6] net_sched: sch_sfq: annotate data-races around q->perturb_period
2025-07-17 12:45 ` [PATCH 5.4.y 1/6] net_sched: sch_sfq: annotate data-races around q->perturb_period Harshit Mogalapalli
@ 2025-07-18 1:34 ` Sasha Levin
0 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2025-07-18 1:34 UTC (permalink / raw)
To: stable; +Cc: Sasha Levin
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected.
No action required from the submitter.
The upstream commit SHA1 provided is correct: a17ef9e6c2c1cf0fc6cd6ca6a9ce525c67d1da7f
WARNING: Author mismatch between patch and upstream commit:
Backport author: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
Commit author: Eric Dumazet <edumazet@google.com>
Results of testing on various branches:
| Branch | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| 5.4 | Success | Success |
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5.4.y 6/6] net_sched: sch_sfq: reject invalid perturb period
2025-07-17 12:45 ` [PATCH 5.4.y 6/6] net_sched: sch_sfq: reject invalid perturb period Harshit Mogalapalli
@ 2025-07-18 1:34 ` Sasha Levin
0 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2025-07-18 1:34 UTC (permalink / raw)
To: stable; +Cc: Sasha Levin
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected.
No action required from the submitter.
The upstream commit SHA1 provided is correct: 7ca52541c05c832d32b112274f81a985101f9ba8
WARNING: Author mismatch between patch and upstream commit:
Backport author: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
Commit author: Eric Dumazet <edumazet@google.com>
Results of testing on various branches:
| Branch | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| 5.4 | Success | Success |
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5.4.y 2/6] net_sched: sch_sfq: handle bigger packets
2025-07-17 12:45 ` [PATCH 5.4.y 2/6] net_sched: sch_sfq: handle bigger packets Harshit Mogalapalli
@ 2025-07-18 1:34 ` Sasha Levin
0 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2025-07-18 1:34 UTC (permalink / raw)
To: stable; +Cc: Sasha Levin
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected.
No action required from the submitter.
The upstream commit SHA1 provided is correct: e4650d7ae4252f67e997a632adfae0dd74d3a99a
WARNING: Author mismatch between patch and upstream commit:
Backport author: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
Commit author: Eric Dumazet <edumazet@google.com>
Results of testing on various branches:
| Branch | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| 5.4 | Success | Success |
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5.4.y 3/6] net_sched: sch_sfq: don't allow 1 packet limit
2025-07-17 12:45 ` [PATCH 5.4.y 3/6] net_sched: sch_sfq: don't allow 1 packet limit Harshit Mogalapalli
@ 2025-07-18 1:34 ` Sasha Levin
0 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2025-07-18 1:34 UTC (permalink / raw)
To: stable; +Cc: Sasha Levin
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected.
No action required from the submitter.
The upstream commit SHA1 provided is correct: 10685681bafce6febb39770f3387621bf5d67d0b
WARNING: Author mismatch between patch and upstream commit:
Backport author: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
Commit author: Octavian Purdila <tavip@google.com>
Results of testing on various branches:
| Branch | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| 5.4 | Success | Success |
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5.4.y 4/6] net_sched: sch_sfq: use a temporary work area for validating configuration
2025-07-17 12:45 ` [PATCH 5.4.y 4/6] net_sched: sch_sfq: use a temporary work area for validating configuration Harshit Mogalapalli
@ 2025-07-18 1:34 ` Sasha Levin
0 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2025-07-18 1:34 UTC (permalink / raw)
To: stable; +Cc: Sasha Levin
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected.
No action required from the submitter.
The upstream commit SHA1 provided is correct: 8c0cea59d40cf6dd13c2950437631dd614fbade6
WARNING: Author mismatch between patch and upstream commit:
Backport author: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
Commit author: Octavian Purdila <tavip@google.com>
Results of testing on various branches:
| Branch | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| 5.4 | Success | Success |
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 5.4.y 5/6] net_sched: sch_sfq: move the limit validation
2025-07-17 12:45 ` [PATCH 5.4.y 5/6] net_sched: sch_sfq: move the limit validation Harshit Mogalapalli
@ 2025-07-18 1:34 ` Sasha Levin
0 siblings, 0 replies; 13+ messages in thread
From: Sasha Levin @ 2025-07-18 1:34 UTC (permalink / raw)
To: stable; +Cc: Sasha Levin
[ Sasha's backport helper bot ]
Hi,
✅ All tests passed successfully. No issues detected.
No action required from the submitter.
The upstream commit SHA1 provided is correct: b3bf8f63e6179076b57c9de660c9f80b5abefe70
WARNING: Author mismatch between patch and upstream commit:
Backport author: Harshit Mogalapalli <harshit.m.mogalapalli@oracle.com>
Commit author: Octavian Purdila <tavip@google.com>
Results of testing on various branches:
| Branch | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| 5.4 | Success | Success |
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2025-07-18 1:34 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-17 12:45 [PATCH 5.4.y 0/6] Backport few sch_sfq fixes Harshit Mogalapalli
2025-07-17 12:45 ` [PATCH 5.4.y 1/6] net_sched: sch_sfq: annotate data-races around q->perturb_period Harshit Mogalapalli
2025-07-18 1:34 ` Sasha Levin
2025-07-17 12:45 ` [PATCH 5.4.y 2/6] net_sched: sch_sfq: handle bigger packets Harshit Mogalapalli
2025-07-18 1:34 ` Sasha Levin
2025-07-17 12:45 ` [PATCH 5.4.y 3/6] net_sched: sch_sfq: don't allow 1 packet limit Harshit Mogalapalli
2025-07-18 1:34 ` Sasha Levin
2025-07-17 12:45 ` [PATCH 5.4.y 4/6] net_sched: sch_sfq: use a temporary work area for validating configuration Harshit Mogalapalli
2025-07-18 1:34 ` Sasha Levin
2025-07-17 12:45 ` [PATCH 5.4.y 5/6] net_sched: sch_sfq: move the limit validation Harshit Mogalapalli
2025-07-18 1:34 ` Sasha Levin
2025-07-17 12:45 ` [PATCH 5.4.y 6/6] net_sched: sch_sfq: reject invalid perturb period Harshit Mogalapalli
2025-07-18 1:34 ` Sasha Levin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).