netdev - Re: [PATCH net v3] net: stmmac: protect updates of 64-bit statistics counters

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: <CANn89iLfgK7zTWMutOycKyVepq=8n26MO_LTsUAy2JdVtXr-_g@mail.gmail.com>
Date: Wed, 6 Mar 2024 11:17:17 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: Petr Tesařík <petr@...arici.cz>
Cc: "Linux regression tracking (Thorsten Leemhuis)" <regressions@...mhuis.info>, 
	Linux regressions mailing list <regressions@...ts.linux.dev>, "David S. Miller" <davem@...emloft.net>, 
	Paolo Abeni <pabeni@...hat.com>, Jakub Kicinski <kuba@...nel.org>, Jisheng Zhang <jszhang@...nel.org>, 
	Alexandre Torgue <alexandre.torgue@...s.st.com>, Jose Abreu <joabreu@...opsys.com>, 
	Maxime Coquelin <mcoquelin.stm32@...il.com>, Chen-Yu Tsai <wens@...e.org>, 
	Jernej Skrabec <jernej.skrabec@...il.com>, Samuel Holland <samuel@...lland.org>, 
	"open list:STMMAC ETHERNET DRIVER" <netdev@...r.kernel.org>, 
	"moderated list:ARM/STM32 ARCHITECTURE" <linux-stm32@...md-mailman.stormreply.com>, 
	"moderated list:ARM/STM32 ARCHITECTURE" <linux-arm-kernel@...ts.infradead.org>, 
	open list <linux-kernel@...r.kernel.org>, 
	"open list:ARM/Allwinner sunXi SoC support" <linux-sunxi@...ts.linux.dev>, Marc Haber <mh+netdev@...schlus.de>, 
	Andrew Lunn <andrew@...n.ch>, Florian Fainelli <f.fainelli@...il.com>, stable@...r.kernel.org, 
	alexis.lothore@...tlin.com, Guenter Roeck <linux@...ck-us.net>
Subject: Re: [PATCH net v3] net: stmmac: protect updates of 64-bit statistics counters

On Wed, Mar 6, 2024 at 11:03 AM Petr Tesařík <petr@...arici.cz> wrote:
>
> On Wed, 6 Mar 2024 10:01:53 +0100
> Petr Tesařík <petr@...arici.cz> wrote:
>
> > On Wed, 6 Mar 2024 09:23:53 +0100
> > "Linux regression tracking (Thorsten Leemhuis)" <regressions@...mhuis.info> wrote:
> >
> > > On 28.02.24 12:03, Petr Tesařík wrote:
> > > > On Wed, 28 Feb 2024 07:19:56 +0100
> > > > "Linux regression tracking (Thorsten Leemhuis)" <regressions@...mhuis.info> wrote:
> > > >
> > > >> Net maintainers, chiming in here, as it seems handling this regression
> > > >> stalled.
> > > > Indeed, I was too busy with sandbox mode...
> > >
> > > Hmm, no reply in the past week to Petr's request for help from someone
> > > with more knowledge about the field. :-/
> > >
> > > So I guess this means that this won't be fixed for 6.8? Unfortunate, but
> > > well, that's how it it sometimes.
> >
> > For the record, I _can_ reproduce lockdep splats on my device, but they
> > don't make any sense to me. They seem to confirm Jisheng Zhang's
> > conclusion that lockdep conflates two locks which should have different
> > lock-classes.
> >
> > So far I have noticed only one issue: the per-cpu syncp's are not
> > initialized. I'll recompile and see if that's what confuses lockdep.
>
> That wasn't the issue. FTR the syncp was in fact initialized, because
> devm_netdev_alloc_pcpu_stats() is a macro that also takes care of the
> initialization of the syncp struct field.
>
> The problem is u64_stats_init().
>
> Commit 9464ca650008 ("net: make u64_stats_init() a function") changed
> it to an inline function. But that's wrong. It uses seqcount_init(),
> which in turn declares:
>
>         static struct lock_class_key __key;
>
> This assumes that each lock gets its own instance. But if
> u64_stats_init() is a function (albeit an inline one), all calls
> within the same file end up using the same instance.
>
> Eric, would it be OK to revert the above-mentioned commit?

Oh, nice !

Well, this would not be a revert, let's keep type safety checks if possible.

Thanks.