Netdev List
 help / color / mirror / Atom feed
* [PATCH v3 net] net/sched: taprio: enforce minimum value for picos_per_byte
@ 2025-07-28 17:31 Takamitsu Iwai
  2025-07-30 17:49 ` Cong Wang
  2025-08-02  0:30 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 4+ messages in thread
From: Takamitsu Iwai @ 2025-07-28 17:31 UTC (permalink / raw)
  To: Vinicius Costa Gomes, Jamal Hadi Salim, Cong Wang, Jiri Pirko
  Cc: netdev, Kuniyuki Iwashima, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman, Vladimir Oltean,
	takamitz, Takamitsu Iwai, syzbot+398e1ee4ca2cac05fddb

Syzbot reported a WARNING in taprio_get_start_time().

When link speed is 470,589 or greater, q->picos_per_byte becomes too
small, causing length_to_duration(q, ETH_ZLEN) to return zero.

This zero value leads to validation failures in fill_sched_entry() and
parse_taprio_schedule(), allowing arbitrary values to be assigned to
entry->interval and cycle_time. As a result, sched->cycle can become zero.

Since SPEED_800000 is the largest defined speed in
include/uapi/linux/ethtool.h, this issue can occur in realistic scenarios.

To ensure length_to_duration() returns a non-zero value for minimum-sized
Ethernet frames (ETH_ZLEN = 60), picos_per_byte must be at least 17
(60 * 17 > PSEC_PER_NSEC which is 1000).

This patch enforces a minimum value of 17 for picos_per_byte when the
calculated value would be lower, and adds a warning message to inform
users that scheduling accuracy may be affected at very high link speeds.

Fixes: fb66df20a720 ("net/sched: taprio: extend minimum interval restriction to entire cycle too")
Reported-by: syzbot+398e1ee4ca2cac05fddb@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=398e1ee4ca2cac05fddb
Signed-off-by: Takamitsu Iwai <takamitz@amazon.co.jp>
---
Changes:
  v3:
    - Remove unnecessary blank line.
    - Add NL_SET_ERR_MSG_FMT_MOD() to show warning directly to the users
      when taprio_set_picos_per_byte() is called from taprio_change().

  v2: https://lore.kernel.org/all/20250726010815.20198-1-takamitz@amazon.co.jp/
    - Add pr_warn() for users to inform link speed is too high for scheduling.
    - Correct fixes tag to indicate appropriate commit.

  v1: https://lore.kernel.org/all/20250724181345.40961-1-takamitz@amazon.co.jp/

 net/sched/sch_taprio.c | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index e759e43ad27e..39b735386996 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -43,6 +43,11 @@ static struct static_key_false taprio_have_working_mqprio;
 #define TAPRIO_SUPPORTED_FLAGS \
 	(TCA_TAPRIO_ATTR_FLAG_TXTIME_ASSIST | TCA_TAPRIO_ATTR_FLAG_FULL_OFFLOAD)
 #define TAPRIO_FLAGS_INVALID U32_MAX
+/* Minimum value for picos_per_byte to ensure non-zero duration
+ * for minimum-sized Ethernet frames (ETH_ZLEN = 60).
+ * 60 * 17 > PSEC_PER_NSEC (1000)
+ */
+#define TAPRIO_PICOS_PER_BYTE_MIN 17
 
 struct sched_entry {
 	/* Durations between this GCL entry and the GCL entry where the
@@ -1284,7 +1289,8 @@ static void taprio_start_sched(struct Qdisc *sch,
 }
 
 static void taprio_set_picos_per_byte(struct net_device *dev,
-				      struct taprio_sched *q)
+				      struct taprio_sched *q,
+				      struct netlink_ext_ack *extack)
 {
 	struct ethtool_link_ksettings ecmd;
 	int speed = SPEED_10;
@@ -1300,6 +1306,15 @@ static void taprio_set_picos_per_byte(struct net_device *dev,
 
 skip:
 	picos_per_byte = (USEC_PER_SEC * 8) / speed;
+	if (picos_per_byte < TAPRIO_PICOS_PER_BYTE_MIN) {
+		if (!extack)
+			pr_warn("Link speed %d is too high. Schedule may be inaccurate.\n",
+				speed);
+		NL_SET_ERR_MSG_FMT_MOD(extack,
+				       "Link speed %d is too high. Schedule may be inaccurate.",
+				       speed);
+		picos_per_byte = TAPRIO_PICOS_PER_BYTE_MIN;
+	}
 
 	atomic64_set(&q->picos_per_byte, picos_per_byte);
 	netdev_dbg(dev, "taprio: set %s's picos_per_byte to: %lld, linkspeed: %d\n",
@@ -1324,7 +1339,7 @@ static int taprio_dev_notifier(struct notifier_block *nb, unsigned long event,
 		if (dev != qdisc_dev(q->root))
 			continue;
 
-		taprio_set_picos_per_byte(dev, q);
+		taprio_set_picos_per_byte(dev, q, NULL);
 
 		stab = rtnl_dereference(q->root->stab);
 
@@ -1844,7 +1859,7 @@ static int taprio_change(struct Qdisc *sch, struct nlattr *opt,
 	q->flags = taprio_flags;
 
 	/* Needed for length_to_duration() during netlink attribute parsing */
-	taprio_set_picos_per_byte(dev, q);
+	taprio_set_picos_per_byte(dev, q, extack);
 
 	err = taprio_parse_mqprio_opt(dev, mqprio, extack, q->flags);
 	if (err < 0)
-- 
2.39.5 (Apple Git-154)


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 net] net/sched: taprio: enforce minimum value for picos_per_byte
  2025-07-28 17:31 [PATCH v3 net] net/sched: taprio: enforce minimum value for picos_per_byte Takamitsu Iwai
@ 2025-07-30 17:49 ` Cong Wang
  2025-07-31 19:27   ` Vinicius Costa Gomes
  2025-08-02  0:30 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 4+ messages in thread
From: Cong Wang @ 2025-07-30 17:49 UTC (permalink / raw)
  To: Takamitsu Iwai
  Cc: Vinicius Costa Gomes, Jamal Hadi Salim, Jiri Pirko, netdev,
	Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, Vladimir Oltean, takamitz,
	syzbot+398e1ee4ca2cac05fddb

On Tue, Jul 29, 2025 at 02:31:49AM +0900, Takamitsu Iwai wrote:
> Syzbot reported a WARNING in taprio_get_start_time().
> 
> When link speed is 470,589 or greater, q->picos_per_byte becomes too
> small, causing length_to_duration(q, ETH_ZLEN) to return zero.
> 
> This zero value leads to validation failures in fill_sched_entry() and
> parse_taprio_schedule(), allowing arbitrary values to be assigned to
> entry->interval and cycle_time. As a result, sched->cycle can become zero.
> 
> Since SPEED_800000 is the largest defined speed in
> include/uapi/linux/ethtool.h, this issue can occur in realistic scenarios.
> 
> To ensure length_to_duration() returns a non-zero value for minimum-sized
> Ethernet frames (ETH_ZLEN = 60), picos_per_byte must be at least 17
> (60 * 17 > PSEC_PER_NSEC which is 1000).
> 
> This patch enforces a minimum value of 17 for picos_per_byte when the
> calculated value would be lower, and adds a warning message to inform
> users that scheduling accuracy may be affected at very high link speeds.

Is it possible to reproduce this with a selftest? If so, please consider
adding one.

Thanks.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 net] net/sched: taprio: enforce minimum value for picos_per_byte
  2025-07-30 17:49 ` Cong Wang
@ 2025-07-31 19:27   ` Vinicius Costa Gomes
  0 siblings, 0 replies; 4+ messages in thread
From: Vinicius Costa Gomes @ 2025-07-31 19:27 UTC (permalink / raw)
  To: Cong Wang, Takamitsu Iwai
  Cc: Jamal Hadi Salim, Jiri Pirko, netdev, Kuniyuki Iwashima,
	David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
	Simon Horman, Vladimir Oltean, takamitz,
	syzbot+398e1ee4ca2cac05fddb

Cong Wang <xiyou.wangcong@gmail.com> writes:

> On Tue, Jul 29, 2025 at 02:31:49AM +0900, Takamitsu Iwai wrote:
>> Syzbot reported a WARNING in taprio_get_start_time().
>> 
>> When link speed is 470,589 or greater, q->picos_per_byte becomes too
>> small, causing length_to_duration(q, ETH_ZLEN) to return zero.
>> 
>> This zero value leads to validation failures in fill_sched_entry() and
>> parse_taprio_schedule(), allowing arbitrary values to be assigned to
>> entry->interval and cycle_time. As a result, sched->cycle can become zero.
>> 
>> Since SPEED_800000 is the largest defined speed in
>> include/uapi/linux/ethtool.h, this issue can occur in realistic scenarios.
>> 
>> To ensure length_to_duration() returns a non-zero value for minimum-sized
>> Ethernet frames (ETH_ZLEN = 60), picos_per_byte must be at least 17
>> (60 * 17 > PSEC_PER_NSEC which is 1000).
>> 
>> This patch enforces a minimum value of 17 for picos_per_byte when the
>> calculated value would be lower, and adds a warning message to inform
>> users that scheduling accuracy may be affected at very high link speeds.
>
> Is it possible to reproduce this with a selftest? If so, please consider
> adding one.

Good idea. From a quick look, it seems that netdevsim doesn't have
support for .{get,set}_link_ksettings(), so I guess it would be
something for later.


Cheers,
-- 
Vinicius

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v3 net] net/sched: taprio: enforce minimum value for picos_per_byte
  2025-07-28 17:31 [PATCH v3 net] net/sched: taprio: enforce minimum value for picos_per_byte Takamitsu Iwai
  2025-07-30 17:49 ` Cong Wang
@ 2025-08-02  0:30 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-08-02  0:30 UTC (permalink / raw)
  To: Takamitsu Iwai
  Cc: vinicius.gomes, jhs, xiyou.wangcong, jiri, netdev, kuniyu, davem,
	edumazet, kuba, pabeni, horms, olteanv, takamitz,
	syzbot+398e1ee4ca2cac05fddb

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Tue, 29 Jul 2025 02:31:49 +0900 you wrote:
> Syzbot reported a WARNING in taprio_get_start_time().
> 
> When link speed is 470,589 or greater, q->picos_per_byte becomes too
> small, causing length_to_duration(q, ETH_ZLEN) to return zero.
> 
> This zero value leads to validation failures in fill_sched_entry() and
> parse_taprio_schedule(), allowing arbitrary values to be assigned to
> entry->interval and cycle_time. As a result, sched->cycle can become zero.
> 
> [...]

Here is the summary with links:
  - [v3,net] net/sched: taprio: enforce minimum value for picos_per_byte
    https://git.kernel.org/netdev/net/c/ae8508b25def

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-08-02  0:29 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-28 17:31 [PATCH v3 net] net/sched: taprio: enforce minimum value for picos_per_byte Takamitsu Iwai
2025-07-30 17:49 ` Cong Wang
2025-07-31 19:27   ` Vinicius Costa Gomes
2025-08-02  0:30 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox