[PATCH 5.15.y 5/5] net_sched: sch_sfq: move the limit validation

[Harshit Mogalapalli]

From: Octavian Purdila <ta...@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 <syzk...@googlegroups.com>
Fixes: 10685681bafc ("net_sched: sch_sfq: don't allow 1 packet limit")
Signed-off-by: Octavian Purdila <ta...@google.com>
Acked-by: Cong Wang <xiyou.w...@gmail.com>
Signed-off-by: David S. Miller <da...@davemloft.net>
(cherry picked from commit b3bf8f63e6179076b57c9de660c9f80b5abefe70)
Signed-off-by: Harshit Mogalapalli <harshit.m....@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 0d916146a5a3..04c3aa446ad3 100644
--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -658,10 +658,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);

@@ -702,6 +698,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

[Harshit Mogalapalli]

index 9a8c9138702a..ce7d658b7ecb 100644

[Eric Dumazet]

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

commit b3bf8f63e6179076b57c9de660c9f80b5abefe70 upstream.

---
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 0d916146a5a39b492a89067e19b81628bd72f5d1..04c3aa446ad3d69c014b06e66795b8dfbc369333 100644

--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -658,10 +658,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);

@@ -702,6 +698,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.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: move the limit validation

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-move-the-limit-validation.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-152673-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:53 -0700
Subject: net_sched: sch_sfq: move the limit validation
To: sta...@vger.kernel.org
Cc: ta...@google.com, edum...@google.com, syzbot <syzk...@googlegroups.com>, Cong Wang <xiyou.w...@gmail.com>, "David S. Miller" <da...@davemloft.net>, 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 | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)

--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -661,10 +661,6 @@ 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);

@@ -705,6 +701,12 @@ static int sfq_change(struct Qdisc *sch,

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;

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: move the limit validation

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-move-the-limit-validation.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-155166-greg=kroa...@vger.kernel.org Fri Jun 20 17:53:20 2025
From: Eric Dumazet <edum...@google.com>
Date: Fri, 20 Jun 2025 15:46:21 +0000

Subject: net_sched: sch_sfq: move the limit validation
To: sta...@vger.kernel.org

Cc: Octavian Purdila <ta...@google.com>, syzbot <syzk...@googlegroups.com>, Cong Wang <xiyou.w...@gmail.com>, "David S. Miller" <da...@davemloft.net>
Message-ID: <20250620154623.3...@google.com>

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

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

--- a/net/sched/sch_sfq.c
+++ b/net/sched/sch_sfq.c
@@ -661,10 +661,6 @@ 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);

@@ -705,6 +701,12 @@ static int sfq_change(struct Qdisc *sch,

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;

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

[Greg Kroah-Hartman]

5.10-stable review patch. If anyone has any objections, please let me know.

------------------

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

[ Upstream commit b3bf8f63e6179076b57c9de660c9f80b5abefe70 ]

Signed-off-by: Harshit Mogalapalli <harshit.m....@oracle.com>

[Greg Kroah-Hartman]

5.15-stable review patch. If anyone has any objections, please let me know.

------------------

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

commit b3bf8f63e6179076b57c9de660c9f80b5abefe70 upstream.