[PATCH] net/sched: act_gate: Limit the max value for cycletime

[PATCH] net/sched: act_gate: Limit the max value for cycletime

[Edward Adam Davis]

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].

When initializing 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>
---
 net/sched/act_gate.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
index fdbfcaa3e2ab..ad5a6d75bbda 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -501,6 +501,8 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
 			cycle = ktime_add_ns(cycle, entry->interval);
 		cycletime = cycle;
 	}
+	if (cycletime > S64_MAX)
+		cycletime = S64_MAX;
 	p->tcfg_cycletime = cycletime;
 	p->tcfg_cycletime_ext = cycletime_ext;
 
-- 
2.43.0

Re: [PATCH] net/sched: act_gate: Limit the max value for cycletime

[Jakub Kicinski]

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

The patch clamps a user supplied TCA_GATE_CYCLE_TIME to S64_MAX in
tcf_gate_init() before storing it in p->tcfg_cycletime, so that the
ktime_t (s64) arithmetic used to compute gate close times does not see a
negative cycle.  It is tagged as the fix for a syzbot soft lockup inside
gate_timer_func().

Closes: https://syzkaller.appspot.com/bug?extid=0054fed3dc9085390f51

> diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
> index fdbfcaa3e2ab..ad5a6d75bbda 100644
> --- a/net/sched/act_gate.c
> +++ b/net/sched/act_gate.c
> @@ -501,6 +501,8 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
>  			cycle = ktime_add_ns(cycle, entry->interval);
>  		cycletime = cycle;
>  	}
> +	if (cycletime > S64_MAX)
> +		cycletime = S64_MAX;

[Severity: High]
This isn't a bug introduced by this patch, since cycletime == S64_MAX
combined with a non-zero basetime already wrapped before it, but does
clamping to S64_MAX actually close the reported soft lockup?

The consumers do not use the cycle value on its own, they compute an
absolute expiry as base + (n + 1) * cycle with plain signed ktime
arithmetic:

net/sched/act_gate.c:gate_get_start_time() {
	...
	cycle = param->tcfg_cycletime;

	n = div64_u64(ktime_sub_ns(now, base), cycle);
	*start = ktime_add_ns(base, (n + 1) * cycle);
}

With cycle == S64_MAX and any non-zero basetime that is not in the
future, div64_u64(now - base, S64_MAX) is 0, so the addition is
base + S64_MAX and wraps negative (for a basetime of 1000 ns, start
becomes S64_MIN + 1000).  gate_start_timer() then takes
min_t(ktime_t, start, expires) and arms the timer with that past
absolute value in HRTIMER_MODE_ABS_SOFT.

gate_timer_func() recomputes the same wrapped value on every invocation:

net/sched/act_gate.c:gate_timer_func() {
	...
	if (ktime_after(now, close_time)) {
		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;
}

Since the expiry never advances past now, doesn't __hrtimer_run_queues()
keep re-running the callback in the same softirq pass, reproducing the
watchdog trace quoted in the changelog?  Only basetime == 0
self-corrects, because 0 + S64_MAX stays positive, which would explain
why the syzbot reproducer no longer trips; adding a base-time of 1000
appears to trip it again.

Two follow-on effects of that loop: gate_timer_func() re-takes
gact->tcf_lock on every iteration, and hrtimer_cancel() in
tcf_gate_cleanup() (and on the replace path in tcf_gate_init()) can
never see the perpetually re-armed timer dequeued, so the params and
entry list are never handed to call_rcu(&p->rcu,
tcf_gate_params_free_rcu).

Note also that TCA_GATE_BASE_TIME is read with a plain nla_get_u64()
and gate_policy declares it as unrestricted NLA_U64, so any
basetime/cycletime pair whose sum exceeds S64_MAX wraps the same way,
for example cycletime = 2^62 with basetime = 3 * 2^62.  Would bounding
the computed expiry instead (basetime + cycletime <= KTIME_MAX), or
rejecting the input in a narrower range as sch_taprio does, be a better
fit?

Related to this, the changelog says:

    When initializing cycletime, ensuring its value does not exceed
    S64_MAX guarantees that the hrtimer can correctly calculate a valid
    expiry time.

Given the wrap in base + (n + 1) * cycle above, is that guarantee
accurate for non-zero basetime?

[Severity: Medium]
Should an out-of-range cycle time be rejected rather than silently
rewritten?

gate_policy declares the attribute with no range restriction:

	[TCA_GATE_CYCLE_TIME]		= { .type = NLA_U64 },

and tcf_gate_dump() echoes the stored value back in the same attribute:

	if (nla_put_u64_64bit(skb, TCA_GATE_CYCLE_TIME,
			      p->tcfg_cycletime, TCA_GATE_PAD))

so a request with cycle-time 0xffffffffffffffff now succeeds, while a
subsequent dump reports S64_MAX.  There is no -EINVAL, no
NL_SET_BAD_ATTR and no extack message on that path.

A later replace that omits the attribute also inherits the rewritten
value:

net/sched/act_gate.c:tcf_gate_init() {
	...
			if (!err && !tb[TCA_GATE_CYCLE_TIME])
				cycletime = cur_p->tcfg_cycletime;
	...
}

Every other invalid input in this file is rejected with -EINVAL plus
extack (interval == 0 in fill_gate_entry(), bad clockid in
gate_clock_resolve(), empty entry list), and the same knob in
sch_taprio is rejected rather than clamped:

net/sched/sch_taprio.c:parse_taprio_schedule() {
	...
		if (cycle < 0 || cycle > INT_MAX) {
			NL_SET_ERR_MSG(extack, "'cycle_time' is too big");
			return -EINVAL;
		}
	...
}

The branch immediately above the new check cannot exceed S64_MAX, since
intervals are non-zero u32 values and roughly 2^31 entries would be
needed, so the check only covers the user supplied attribute.  Could it
move to the parse stage and return -EINVAL with NL_SET_BAD_ATTR on
TCA_GATE_CYCLE_TIME instead?

>  	p->tcfg_cycletime = cycletime;
>  	p->tcfg_cycletime_ext = cycletime_ext;
>
-- 
pw-bot: cr

[PATCH v2] net/sched: act_gate: Limit the max value for cycletime

[Edward Adam Davis]

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].

When initializing cycletime, ensuring its value does not exceed INT_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

 net/sched/act_gate.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
index fdbfcaa3e2ab..30bcf173274c 100644
--- a/net/sched/act_gate.c
+++ b/net/sched/act_gate.c
@@ -501,6 +501,14 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
 			cycle = ktime_add_ns(cycle, entry->interval);
 		cycletime = cycle;
 	}
+
+	if (cycletime < 0 || cycletime > INT_MAX) {
+		NL_SET_ERR_MSG(extack, "'cycle_time' is too big");
+		err = -EINVAL;
+		spin_unlock_bh(&gact->tcf_lock);
+		goto err_free;
+	}
+
 	p->tcfg_cycletime = cycletime;
 	p->tcfg_cycletime_ext = cycletime_ext;
 
-- 
2.43.0

Re: [PATCH v2] net/sched: act_gate: Limit the max value for cycletime

[Jamal Hadi Salim]

On Sun, Aug 23, 2026 at 12:07 AM Edward Adam Davis <eadavis@qq.com> wrote:
>
> 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.
>

Review the sashiko feedback. At least two of those concerns look legit
and need to be addressed:
1) complaint about timer disarm on replace 2) INT_MAX being too narrow

cheers,
jamal
> 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].
>
> When initializing cycletime, ensuring its value does not exceed INT_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
>
>  net/sched/act_gate.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/net/sched/act_gate.c b/net/sched/act_gate.c
> index fdbfcaa3e2ab..30bcf173274c 100644
> --- a/net/sched/act_gate.c
> +++ b/net/sched/act_gate.c
> @@ -501,6 +501,14 @@ static int tcf_gate_init(struct net *net, struct nlattr *nla,
>                         cycle = ktime_add_ns(cycle, entry->interval);
>                 cycletime = cycle;
>         }
> +
> +       if (cycletime < 0 || cycletime > INT_MAX) {
> +               NL_SET_ERR_MSG(extack, "'cycle_time' is too big");
> +               err = -EINVAL;
> +               spin_unlock_bh(&gact->tcf_lock);
> +               goto err_free;
> +       }
> +
>         p->tcfg_cycletime = cycletime;
>         p->tcfg_cycletime_ext = cycletime_ext;
>
> --
> 2.43.0
>

Re: [PATCH v2] net/sched: act_gate: Limit the max value for cycletime

[Edward Adam Davis]

On Tue, 25 Aug 2026 04:25:04 -0400, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On Sun, Aug 23, 2026 at 12:07 AM Edward Adam Davis <eadavis@qq.com> wrote:
> >
> > 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.
> >
> 
> Review the sashiko feedback. At least two of those concerns look legit
> and need to be addressed:
> 1) complaint about timer disarm on replace 2) INT_MAX being too narrow
I haven't received any feedback regarding sashiko, and I didn't quite
understand the points made in item 1); could you please provide more
details?

Regarding item 2), if INT_MAX is too narrow, do you have a suitable value
to recommend? S64_MAX?

cheers,
Edward

Re: [PATCH v2] net/sched: act_gate: Limit the max value for cycletime

[Jamal Hadi Salim]

On Tue, Aug 25, 2026 at 5:01 AM Edward Adam Davis <eadavis@qq.com> wrote:
>
> On Tue, 25 Aug 2026 04:25:04 -0400, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > On Sun, Aug 23, 2026 at 12:07 AM Edward Adam Davis <eadavis@qq.com> wrote:
> > >
> > > 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.
> > >
> >
> > Review the sashiko feedback. At least two of those concerns look legit
> > and need to be addressed:
> > 1) complaint about timer disarm on replace 2) INT_MAX being too narrow
> I haven't received any feedback regarding sashiko, and I didn't quite
> understand the points made in item 1); could you please provide more
> details?

You should always look at patchwork for reviews from the AIs - i just
happened to have cycles and peeked and even my responses are best
effort. Some maintainers forward AI reviews on a best-effort basis
(Jakub forwarded you the v1 review), so you may end up getting radio
silence if nobody has time. So, going forward, the first line of
defense is to look at patchwork 24 hours + after you post your patch.
Address those by sending a new version or rebut them on the list.
> Regarding item 2), if INT_MAX is too narrow, do you have a suitable value
> to recommend? S64_MAX?

Handwave: The safe bound is one that keeps base + cycletime <= KTIME_MAX.
To be verbose per sashiko:
cycletime is an unrestricted NLA_U64 consumed as ktime_t (s64). A 200
s cycle time does not overflow any s64 arithmetic, but your v2 rejects
it. tdc test a721
(tools/testing/selftests/tc-testing/tc-tests/actions/gate.json) uses
cycle-time 200000000000ns and expects success; your v2 breaks it.
sch_taprio uses INT_MAX but its semantics/tests differ. S64_MAX is the
actual overflow boundary (U64_MAX is what makes the s64 go negative);
but even S64_MAX + a non-zero basetime wraps in base + (n+1)*cycle

> cheers,
> Edward
>

Re: [PATCH v2] net/sched: act_gate: Limit the max value for cycletime

[Edward Adam Davis]

On Tue, 25 Aug 2026 05:31:28 -0400, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> On Tue, Aug 25, 2026 at 5:01 AM Edward Adam Davis <eadavis@qq.com> wrote:
> >
> > On Tue, 25 Aug 2026 04:25:04 -0400, Jamal Hadi Salim <jhs@mojatatu.com> wrote:
> > > On Sun, Aug 23, 2026 at 12:07 AM Edward Adam Davis <eadavis@qq.com> wrote:
> > > >
> > > > 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.
> > > >
> > >
> > > Review the sashiko feedback. At least two of those concerns look legit
> > > and need to be addressed:
> > > 1) complaint about timer disarm on replace 2) INT_MAX being too narrow
> > I haven't received any feedback regarding sashiko, and I didn't quite
> > understand the points made in item 1); could you please provide more
> > details?
> 
> You should always look at patchwork for reviews from the AIs - i just
> happened to have cycles and peeked and even my responses are best
> effort. Some maintainers forward AI reviews on a best-effort basis
> (Jakub forwarded you the v1 review), so you may end up getting radio
> silence if nobody has time. So, going forward, the first line of
> defense is to look at patchwork 24 hours + after you post your patch.
> Address those by sending a new version or rebut them on the list.
> > Regarding item 2), if INT_MAX is too narrow, do you have a suitable value
> > to recommend? S64_MAX?
> 
> Handwave: The safe bound is one that keeps base + cycletime <= KTIME_MAX.
> To be verbose per sashiko:
> cycletime is an unrestricted NLA_U64 consumed as ktime_t (s64). A 200
> s cycle time does not overflow any s64 arithmetic, but your v2 rejects
> it. tdc test a721
> (tools/testing/selftests/tc-testing/tc-tests/actions/gate.json) uses
> cycle-time 200000000000ns and expects success; your v2 breaks it.
> sch_taprio uses INT_MAX but its semantics/tests differ. S64_MAX is the
> actual overflow boundary (U64_MAX is what makes the s64 go negative);
> but even S64_MAX + a non-zero basetime wraps in base + (n+1)*cycle
Got it, thanks.

BR,
Edward

[PATCH v3] net/sched: act_gate: Limit the max value for cycletime

[Edward Adam Davis]

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

Re: [PATCH v3] net/sched: act_gate: Limit the max value for cycletime

[Jakub Kicinski]

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

Re: [PATCH v3] net/sched: act_gate: Limit the max value for cycletime

[Jakub Kicinski]

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.