netdev - Re: [RFC PATCH] net: stmmac: Fix the problem about interrupt storm

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: <7732873.EvYhyI6sBW@fw-rgant>
Date: Tue, 12 Nov 2024 16:12:17 +0100
From: Romain Gantois <romain.gantois@...tlin.com>
To: cathycai0714@...il.com, Alexandre Torgue <alexandre.torgue@...s.st.com>,
 Jose Abreu <joabreu@...opsys.com>,
 Giuseppe Cavallaro <peppe.cavallaro@...com>,
 Avi Fishman <avifishman70@...il.com>
Cc: cathy.cai@...soc.com, cixi.geng1@...soc.com,
 David Miller <davem@...emloft.net>, edumazet@...gle.com, kuba@...nel.org,
 Linux ARM <linux-arm-kernel@...ts.infradead.org>,
 Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
 linux-stm32@...md-mailman.stormreply.com, mcoquelin.stm32@...il.com,
 Network Development <netdev@...r.kernel.org>, pabeni@...hat.com,
 wade.shu@...soc.com, xuewen.yan94@...il.com, zhiguo.niu@...soc.com,
 Alexandre Torgue <alexandre.torgue@...com>,
 Murali <murali.somarouthu@...l.com>, Tomer Maimon <tmaimon77@...il.com>,
 "Silva, L Antonio" <Luis.A.Silva@...l.com>,
 Arias Pablo <Pablo_Arias@...l.com>,
 Somarouthu Murali <Murali_Somarouthu@...l.com>, uri.trichter@...oton.com
Subject: Re: [RFC PATCH] net: stmmac: Fix the problem about interrupt storm

Hello,

On dimanche 3 novembre 2024 20:00:54 heure normale d’Europe centrale Avi 
Fishman wrote:
> Hi all,
> 
...
> >  Yes. It could also happen between the dev_open() and
> > 
> > clear_bit(STMMAC_DOWN) calls.
> > Although we did not reproduce this scenario, it should have happened
> > if we had increased
> > the number of test samples. In addition, I found that other people had
> > similar problems before.
> > The link is:
> > https://lore.kernel.org/all/20210208140820.10410-11-Sergey.Semin@baikalele
> > ctronics.ru/> 
> > > Moreover, it seems strange to me that stmmac_interrupt()
> > > unconditionnally
> > > ignores interrupts when the driver is in STMMAC_DOWN state. This seems
> > > like
> > > dangerous behaviour, since it could cause IRQ storm issues whenever
> > > something in the driver sets this state. I'm not too familiar with the
> > > interrupt handling in this driver, but maybe stmmac_interrupt() could
> > > clear interrupts unconditionnally in the STMMAC_DOWN state?
> > 
> > Clear interrupts unconditionally in the STMMAC_DOWN state directly
> > certainly won't cause this problem.
> > This may be too rough, maybe this design has other considerations.
> 
> But then after the dev_open() you might miss interrupt, no?

Indeed, but in any case, unconditionally returning from an IRQ handler without 
clearing any interrupt flags seems like very strange behavior to me.

Disabling and reenabling interrupts as you suggested does seem like a
good solution for this particular scenario, but it doesn't solve the more
general issue of the dangerous way stmmac_interrupt handles this.

Maybe the setting and clearing of this STMMAC_DOWN bit should
be wrapped in some kind of handler which also disables all interrupts?

Best Regards

-- 
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com