netdev - Re: [PATCH net] net: stmmac: prevent division by 0 in stmmac_init_tstamp

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] [day] [month] [year] [list]

Message-ID: <877641ca-aadb-4510-9ed7-cc23cf666653@omp.ru>
Date: Mon, 8 Sep 2025 22:39:35 +0300
From: Sergey Shtylyov <s.shtylyov@....ru>
To: Andrew Lunn <andrew@...n.ch>
CC: Andrew Lunn <andrew+netdev@...n.ch>, "David S. Miller"
	<davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski
	<kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Maxime Coquelin
	<mcoquelin.stm32@...il.com>, Alexandre Torgue <alexandre.torgue@...s.st.com>,
	<netdev@...r.kernel.org>, <linux-stm32@...md-mailman.stormreply.com>,
	<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH net] net: stmmac: prevent division by 0 in
 stmmac_init_tstamp_counter()

On 9/8/25 7:47 PM, Andrew Lunn wrote:

>> In stmmac_init_tstamp_counter(), the sec_inc variable is initialized to 0,
>> and if stmmac_config_sub_second_increment() fails to set it to some non-0
>> value, the following div_u64() call would cause a kernel oops (because of
>> the divide error exception).  Let's check sec_inc for 0 before dividing by
>> it and just return -EINVAL if so...
>>
>> Found by Linux Verification Center (linuxtesting.org) with the Svace static
>> analysis tool.
>>
>> Fixes: df103170854e ("net: stmmac: Avoid sometimes uninitialized Clang warnings")
>> Signed-off-by: Sergey Shtylyov <s.shtylyov@....ru>
>>
>> ---
>> The patch is against the master branch of Linus Torvalds' linux.git repo.
> 
> Wrong tree. Please see:
> 
> https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html

   Well, formerly being a reviewer for the Renesas drivers (before Greg KH
threw me out of MAINTAINERS last year), I kinda know this! :-)
   The real problem is that I've lost the ability to pull from git.kernel.org
(first using git:// protocol and later using https:// as well)... Sometimes
the ability used to return (along with Facebook/LinkedIn -- which are actually
blocked in .ru) but that hasn't happened for more than a week now. I can now
pull from Linus' tree at github. I strongly suspect some interaction between
the .ru blocking and the Anubis program that is now used on git.kernel.org --
I've also lost access to lore.kernel.org (sometimes it works -- but not now)
and also elixir.bootlin.com, both of which seem to use the darn Anubis as
well... :-(

> This also needs reviewing by somebody who know the STMMAC
> hardware. There is a comment:
> 
> 	/* For GMAC3.x, 4.x versions, in "fine adjustement mode" set sub-second
                                               ^ Look, a typo! :-)

> 	 * increment to twice the number of nanoseconds of a clock cycle.
> 	 * The calculation of the default_addend value by the caller will set it
> 	 * to mid-range = 2^31 when the remainder of this division is zero,
> 	 * which will make the accumulator overflow once every 2 ptp_clock
> 	 * cycles, adding twice the number of nanoseconds of a clock cycle :
> 	 * 2000000000ULL / ptp_clock.
> 
> So i'm wondering if the subsecond adjustment is sufficient, the
> sec_inc might be zero, and rather than returning an error, the
> hardware just needs programming differently?

   Sorry, I don't readily see how the data var in config_sub_second_increment()
can wrap to 0. I agree that a look of a more knowledgeable person would be good
though... :-)

>     Andrew

[...]

MBR, Sergey