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

[PATCH v1 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 470589 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.

Fixes: 68ce6688a5ba ("net: sched: taprio: Fix potential integer overflow in taprio_set_picos_per_byte")
Reported-by: syzbot+398e1ee4ca2cac05fddb@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=398e1ee4ca2cac05fddb
Signed-off-by: Takamitsu Iwai <takamitz@amazon.co.jp>
---
 net/sched/sch_taprio.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
index 2b14c81a87e5..bcfdb9446657 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
@@ -1299,7 +1305,7 @@ static void taprio_set_picos_per_byte(struct net_device *dev,
 		speed = ecmd.base.speed;
 
 skip:
-	picos_per_byte = (USEC_PER_SEC * 8) / speed;
+	picos_per_byte = max((USEC_PER_SEC * 8) / speed, 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 v1 net] net/sched: taprio: enforce minimum value for picos_per_byte

[Vinicius Costa Gomes]

Takamitsu Iwai <takamitz@amazon.co.jp> writes:

> Syzbot reported a WARNING in taprio_get_start_time().
>
> When link speed is 470589 or greater, q->picos_per_byte becomes too
> small, causing length_to_duration(q, ETH_ZLEN) to return zero.
>

When I chose the unit picoseconds, I remember thinking "nobody would
consider using taprio with over 400Gbs" :-)

> 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.
>
> Fixes: 68ce6688a5ba ("net: sched: taprio: Fix potential integer overflow in taprio_set_picos_per_byte")
> Reported-by: syzbot+398e1ee4ca2cac05fddb@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=398e1ee4ca2cac05fddb
> Signed-off-by: Takamitsu Iwai <takamitz@amazon.co.jp>
> ---
>  net/sched/sch_taprio.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/net/sched/sch_taprio.c b/net/sched/sch_taprio.c
> index 2b14c81a87e5..bcfdb9446657 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
> @@ -1299,7 +1305,7 @@ static void taprio_set_picos_per_byte(struct net_device *dev,
>  		speed = ecmd.base.speed;
>  
>  skip:
> -	picos_per_byte = (USEC_PER_SEC * 8) / speed;
> +	picos_per_byte = max((USEC_PER_SEC * 8) / speed,
>  TAPRIO_PICOS_PER_BYTE_MIN);

Thinking if it's worth displaying an error to the user here? something
like "Linkspeed %d is greater than what can be tracked, schedule may be
inacurate". I am worried that at this point, taprio won't be able to
obey the configured schedule and the user should know about that.

If we see people complaining about it in the real world, we can change
the units, or do something else.

>  
>  	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)
>


Cheers,
-- 
Vinicius

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

[Takamitsu Iwai]

On 2025/07/25, 3:58, "Vinicius Costa Gomes" <vinicius.gomes@intel.com <mailto:vinicius.gomes@intel.com>> wrote:
> Takamitsu Iwai <takamitz@amazon.co.jp <mailto:takamitz@amazon.co.jp>> writes:
> > @@ -1299,7 +1305,7 @@ static void taprio_set_picos_per_byte(struct net_device *dev,
> > speed = ecmd.base.speed;
> >
> > skip:
> > - picos_per_byte = (USEC_PER_SEC * 8) / speed;
> > + picos_per_byte = max((USEC_PER_SEC * 8) / speed,
> > TAPRIO_PICOS_PER_BYTE_MIN);
> 
> 
> Thinking if it's worth displaying an error to the user here? something
> like "Linkspeed %d is greater than what can be tracked, schedule may be
> inacurate". I am worried that at this point, taprio won't be able to
> obey the configured schedule and the user should know about that.
> 
> 
> If we see people complaining about it in the real world, we can change
> the units, or do something else.

Thank you for your feedback on this patch.

I agree with adding message about this here. I'll revise the patch to add 
a pr_warn() message when the calculated picos_per_byte would be below the
minimum threshold.


Thanks.
Takamitsu