net/sched/act_gate.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
If the user passes a cycletime value of 0xFFFFFFFFFFFFFFFFULL,
an overflow occurs during the assignment of cycle in gate_timer_func():
cycle = p->tcfg_cycletime; // overflow, cycle = -1
Since the local variable cycle is declared as ktime_t (i.e., s64),
the assignment overflows.
This leads to an incorrect calculation of the close_time value.
Ultimately, the new hrtimer expiry time becomes less than now, causing
__hrtimer_run_queues() to execute the "timer callback" for an excessively
long period, which triggers a soft lockup. [1]
Another factor is that the passed interval value is 1; while this accelerates
the problematic progression of close_time, it is not the decisive factor in
the issue described in [1].
Modify the cycletime range in the policy to (0, S64_MAX), when parsing
cycletime, ensuring its value does not exceed S64_MAX guarantees that the
hrtimer can correctly calculate a valid expiry time.
[1]
watchdog: BUG: soft lockup - CPU#1 stuck for 3s! [syz-executor291:5020]
pc : seqcount_lockdep_reader_access+0xd8/0xf8 include/linux/seqlock.h:76
Call trace:
arch_local_irq_restore arch/arm64/include/asm/irqflags.h:195 [inline] (P)
seqcount_lockdep_reader_access+0xd8/0xf8 include/linux/seqlock.h:75 (P)
ktime_get+0x68/0x218 kernel/time/timekeeping.c:971
gate_get_time+0x1c/0xa4 net/sched/act_gate.c:23
gate_timer_func+0x1a8/0x390 net/sched/act_gate.c:101
__run_hrtimer kernel/time/hrtimer.c:2032 [inline]
__hrtimer_run_queues+0x314/0xbe0 kernel/time/hrtimer.c:2096
hrtimer_run_softirq+0x15c/0x21c kernel/time/hrtimer.c:2113
handle_softirqs+0x2ec/0xd98 kernel/softirq.c:622
__do_softirq+0x14/0x20 kernel/softirq.c:656
____do_softirq+0x14/0x20 arch/arm64/kernel/irq.c:78
call_on_irq_stack+0x30/0x48 arch/arm64/kernel/entry.S:885
do_softirq_own_stack+0x20/0x2c arch/arm64/kernel/irq.c:83
invoke_softirq kernel/softirq.c:503 [inline]
__irq_exit_rcu+0x1ac/0x428 kernel/softirq.c:735
irq_exit_rcu+0x14/0x84 kernel/softirq.c:752
__el1_irq arch/arm64/kernel/entry-common.c:531 [inline]
el1_interrupt+0x40/0x60 arch/arm64/kernel/entry-common.c:543
el1h_64_irq_handler+0x18/0x24 arch/arm64/kernel/entry-common.c:548
el1h_64_irq+0x6c/0x70 arch/arm64/kernel/entry.S:586
__daif_local_irq_enable arch/arm64/include/asm/irqflags.h:26 [inline] (P)
arch_local_irq_enable arch/arm64/include/asm/irqflags.h:48 [inline] (P)
__local_bh_enable_ip+0x1f0/0x35c kernel/softirq.c:455 (P)
local_bh_enable include/linux/bottom_half.h:33 [inline]
__alloc_skb+0x1c8/0x610 net/core/skbuff.c:699
alloc_skb include/linux/skbuff.h:1384 [inline]
alloc_skb_with_frags+0xb8/0x690 net/core/skbuff.c:6775
sock_alloc_send_pskb+0x740/0x850 net/core/sock.c:3012
unix_dgram_sendmsg+0x434/0x1078 net/unix/af_unix.c:2137
sock_sendmsg_nosec net/socket.c:775 [inline]
Fixes: a51c328df310 ("net: qos: introduce a gate control flow action")
Reported-by: syzbot+0054fed3dc9085390f51@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0054fed3dc9085390f51
Tested-by: syzbot+0054fed3dc9085390f51@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
v1 -> v2: return -EINVAL with NL_SET_BAD_ATTR
v2 -> v3: using policy to limit cycletime range
net/sched/act_gate.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
index fdbfcaa3e2ab..4c4a0f80dec1 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -166,13 +166,19 @@ static const struct nla_policy entry_policy[TCA_GATE_ENTRY_MAX + 1] = {
[TCA_GATE_ENTRY_MAX_OCTETS] = { .type = NLA_S32 },
};
+static const struct netlink_range_validation_signed gate_cycle_time_range = {
+ .min = 0,
+ .max = S64_MAX,
+};
+
static const struct nla_policy gate_policy[TCA_GATE_MAX + 1] = {
[TCA_GATE_PARMS] =
NLA_POLICY_EXACT_LEN(sizeof(struct tc_gate)),
[TCA_GATE_PRIORITY] = { .type = NLA_S32 },
[TCA_GATE_ENTRY_LIST] = { .type = NLA_NESTED },
[TCA_GATE_BASE_TIME] = { .type = NLA_U64 },
- [TCA_GATE_CYCLE_TIME] = { .type = NLA_U64 },
+ [TCA_GATE_CYCLE_TIME] =
+ NLA_POLICY_FULL_RANGE_SIGNED(NLA_S64, &gate_cycle_time_range),
[TCA_GATE_CYCLE_TIME_EXT] = { .type = NLA_U64 },
[TCA_GATE_FLAGS] = { .type = NLA_U32 },
[TCA_GATE_CLOCKID] = { .type = NLA_S32 },
@@ -501,6 +507,7 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
cycle = ktime_add_ns(cycle, entry->interval);
cycletime = cycle;
}
+
p->tcfg_cycletime = cycletime;
p->tcfg_cycletime_ext = cycletime_ext;
--
2.43.0
This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
---
net/sched: act_gate: Limit the max value for cycletime
This changes the act_gate netlink policy so that TCA_GATE_CYCLE_TIME is
validated with NLA_POLICY_FULL_RANGE_SIGNED(NLA_S64) against a range of
0 to S64_MAX, with the intent of preventing an s64 overflow in
gate_timer_func() that led to a soft lockup reported by syzbot.
Closes: https://syzkaller.appspot.com/bug?extid=0054fed3dc9085390f51
A couple of questions on the changelog and on the diff below.
> Modify the cycletime range in the policy to (0, S64_MAX), when parsing
> cycletime, ensuring its value does not exceed S64_MAX guarantees that the
> hrtimer can correctly calculate a valid expiry time.
[Severity: Low]
Should this read [0, S64_MAX] instead of (0, S64_MAX)?
gate_cycle_time_range uses .min = 0 and the range check in
nla_validate_int_range_signed() is inclusive, so a cycletime of 0 is still
accepted, and it keeps its special meaning in tcf_gate_init():
if (!cycletime) {
struct tcfg_gate_entry *entry;
ktime_t cycle = 0;
list_for_each_entry(entry, &p->entries, list)
cycle = ktime_add_ns(cycle, entry->interval);
cycletime = cycle;
}
As written, the open interval wording suggests 0 is now rejected, which a
backporter might rely on.
> diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
> index fdbfcaa3e2ab9..4c4a0f80dec1f 100644
> --- a/net/sched/act_gate.c
> +++ b/net/sched/act_gate.c
> @@ -166,13 +166,19 @@ static const struct nla_policy entry_policy[TCA_GATE_ENTRY_MAX + 1] = {
> [TCA_GATE_ENTRY_MAX_OCTETS] = { .type = NLA_S32 },
> };
>
> +static const struct netlink_range_validation_signed gate_cycle_time_range = {
> + .min = 0,
> + .max = S64_MAX,
> +};
> +
> static const struct nla_policy gate_policy[TCA_GATE_MAX + 1] = {
> [TCA_GATE_PARMS] =
> NLA_POLICY_EXACT_LEN(sizeof(struct tc_gate)),
> [TCA_GATE_PRIORITY] = { .type = NLA_S32 },
> [TCA_GATE_ENTRY_LIST] = { .type = NLA_NESTED },
> [TCA_GATE_BASE_TIME] = { .type = NLA_U64 },
> - [TCA_GATE_CYCLE_TIME] = { .type = NLA_U64 },
> + [TCA_GATE_CYCLE_TIME] =
> + NLA_POLICY_FULL_RANGE_SIGNED(NLA_S64, &gate_cycle_time_range),
[Severity: High]
Does capping cycletime at S64_MAX actually remove the soft lockup?
The expiry is not computed from cycletime alone. In gate_get_start_time():
base = ns_to_ktime(param->tcfg_basetime);
now = gate_get_time(gact);
if (ktime_after(base, now)) {
*start = base;
return;
}
cycle = param->tcfg_cycletime;
n = div64_u64(ktime_sub_ns(now, base), cycle);
*start = ktime_add_ns(base, (n + 1) * cycle);
TCA_GATE_BASE_TIME is left as { .type = NLA_U64 } with no range, so with
basetime = 1 and cycletime = S64_MAX (both accepted by the new policy)
n becomes 0 and the addition is 1 + (2^63 - 1), which wraps to S64_MIN
since ktime_add_ns() is plain 64-bit addition.
gate_start_timer() only clamps downward:
start = min_t(ktime_t, start, expires);
hrtimer_start(&gact->hitimer, start, HRTIMER_MODE_ABS_SOFT);
so the wrapped negative value is armed as an absolute expiry in the past.
The catch-up branch of gate_timer_func() then recomputes the same value on
every invocation:
if (ktime_after(now, close_time)) {
ktime_t cycle, base;
u64 n;
cycle = p->tcfg_cycletime;
base = ns_to_ktime(p->tcfg_basetime);
n = div64_u64(ktime_sub_ns(now, base), cycle);
close_time = ktime_add_ns(base, (n + 1) * cycle);
}
...
hrtimer_set_expires(&gact->hitimer, close_time);
...
return HRTIMER_RESTART;
which looks like the same "gate_timer_func in hrtimer_run_softirq" pattern
from the report, reachable via RTM_NEWACTION with basetime = 1,
cycletime = S64_MAX and one entry with a non-zero interval. A wide band of
policy-legal pairs wraps the same way, for example cycletime = S64_MAX - 10
with basetime = 10^12.
Would bounding the whole base + (n + 1) * cycle arithmetic be needed here,
the way sch_taprio.c does it?
static const struct netlink_range_validation_signed taprio_cycle_time_range = {
.min = 0,
.max = INT_MAX,
};
--
pw-bot: cr
On Tue, 25 Aug 2026 20:13:02 +0800 Edward Adam Davis wrote:
> static const struct nla_policy gate_policy[TCA_GATE_MAX + 1] = {
> [TCA_GATE_PARMS] =
> NLA_POLICY_EXACT_LEN(sizeof(struct tc_gate)),
> [TCA_GATE_PRIORITY] = { .type = NLA_S32 },
> [TCA_GATE_ENTRY_LIST] = { .type = NLA_NESTED },
> [TCA_GATE_BASE_TIME] = { .type = NLA_U64 },
> - [TCA_GATE_CYCLE_TIME] = { .type = NLA_U64 },
> + [TCA_GATE_CYCLE_TIME] =
> + NLA_POLICY_FULL_RANGE_SIGNED(NLA_S64, &gate_cycle_time_range),
Why are you changing the type of the attr? S64_MAX is perfectly fine as
range for NLA_U64
> [TCA_GATE_CYCLE_TIME_EXT] = { .type = NLA_U64 },
> [TCA_GATE_FLAGS] = { .type = NLA_U32 },
> [TCA_GATE_CLOCKID] = { .type = NLA_S32 },
> @@ -501,6 +507,7 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
> cycle = ktime_add_ns(cycle, entry->interval);
> cycletime = cycle;
> }
> +
> p->tcfg_cycletime = cycletime;
> p->tcfg_cycletime_ext = cycletime_ext;
Unrelated noise, please pay more attention.
© 2016 - 2026 Red Hat, Inc.