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

Edward Adam Davis posted 1 patch 1 month ago
net/sched/act_gate.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
[PATCH v3] net/sched: act_gate: Limit the max value for cycletime
Posted by Edward Adam Davis 1 month ago
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
Posted by Jakub Kicinski 1 month ago
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
Posted by Jakub Kicinski 1 month ago
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.