[PATCH v2 net] net/sched: taprio: enforce minimum value for picos_per_byte

[PATCH v2 net] net/sched: taprio: enforce minimum value for picos_per_byte

[Takamitsu Iwai]

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:
  v2:
    - Add warning 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 | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 2b14c81a87e5..55c8ca4f946d 100644
--- a/net/sched/sch_taprio.c
+++ b/net/sched/sch_taprio.c
@@ -43,6 +43,12 @@ 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
@@ -1300,6 +1306,11 @@ 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) {
+		pr_warn("Link speed %d is too high. Schedule may be inaccurate.\n",
+			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",
-- 
2.39.5 (Apple Git-154)

Re: [PATCH v2 net] net/sched: taprio: enforce minimum value for picos_per_byte

[Jakub Kicinski]

On Sat, 26 Jul 2025 10:08:15 +0900 Takamitsu Iwai wrote:
>  #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
> +

unnecessary new line

>  struct sched_entry {
>  	/* Durations between this GCL entry and the GCL entry where the
> @@ -1300,6 +1306,11 @@ 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) {
> +		pr_warn("Link speed %d is too high. Schedule may be inaccurate.\n",
> +			speed);
> +		picos_per_byte = TAPRIO_PICOS_PER_BYTE_MIN;

for the path coming in from taprio_change() you should use the extack
to report the warning (if return value is 0 but extack was set CLIs
will print that message as a warning directly to the user)
-- 
pw-bot: cr

Re: [PATCH v2 net] net/sched: taprio: enforce minimum value for picos_per_byte

[Takamitsu Iwai]

>On 2025/07/27, 3:38, "Jakub Kicinski" <kuba@kernel.org <mailto:kuba@kernel.org>> wrote:
>>  struct sched_entry {
>>       /* Durations between this GCL entry and the GCL entry where the
>> @@ -1300,6 +1306,11 @@ 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) {
>> +             pr_warn("Link speed %d is too high. Schedule may be inaccurate.\n",
>> +                     speed);
>> +             picos_per_byte = TAPRIO_PICOS_PER_BYTE_MIN;
>
>for the path coming in from taprio_change() you should use the extack
>to report the warning (if return value is 0 but extack was set CLIs
>will print that message as a warning directly to the user)


Thank you for your review.

I decided to use pr_warn() only because taprio_set_picos_per_byte() is
called from two different paths:

1. From taprio_change() where extack is available
2. From taprio_dev_notifier() where extack is not available

I plan to modify the patch to handle both cases by:
1. Adding an optional extack parameter to taprio_set_picos_per_byte()
2. Using NL_SET_ERR_MSG_FMT_MOD when extack is provided
3. Falling back to pr_warn() only when extack is NULL

I'll submit an updated version with these changes.


Thanks,
Takamitsu