netdev - Re: [PATCH v1 net] net/sched: taprio: enforce minimum value for picos_per

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87ecu5e7r0.fsf@intel.com>
Date: Thu, 24 Jul 2025 11:58:11 -0700
From: Vinicius Costa Gomes <vinicius.gomes@...el.com>
To: Takamitsu Iwai <takamitz@...zon.co.jp>, Jamal Hadi Salim
 <jhs@...atatu.com>, Cong Wang <xiyou.wangcong@...il.com>, Jiri Pirko
 <jiri@...nulli.us>
Cc: netdev@...r.kernel.org, Kuniyuki Iwashima <kuniyu@...gle.com>, "David S.
 Miller" <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub
 Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman
 <horms@...nel.org>, Vladimir Oltean <olteanv@...il.com>,
 takamitz@...zon.com, Takamitsu Iwai <takamitz@...zon.co.jp>,
 syzbot+398e1ee4ca2cac05fddb@...kaller.appspotmail.com
Subject: Re: [PATCH v1 net] net/sched: taprio: enforce minimum value for
 picos_per_byte

Takamitsu Iwai <takamitz@...zon.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@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=398e1ee4ca2cac05fddb
> Signed-off-by: Takamitsu Iwai <takamitz@...zon.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