netdev - Re: [PATCH net 4/4] sh_eth: Fix serialisation of interrupt disable with interrupt & NAPI handlers

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]

Date:	Thu, 22 Jan 2015 17:59:41 +0000
From:	Ben Hutchings <ben.hutchings@...ethink.co.uk>
To:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
Cc:	"David S.Miller" <davem@...emloft.net>, netdev@...r.kernel.org,
	linux-kernel@...ts.codethink.co.uk,
	Nobuhiro Iwamatsu <nobuhiro.iwamatsu.yj@...esas.com>,
	Mitsuhiro Kimura <mitsuhiro.kimura.kc@...esas.com>,
	Hisashi Nakamura <hisashi.nakamura.ak@...esas.com>,
	Yoshihiro Kaneko <ykaneko0929@...il.com>
Subject: Re: [PATCH net 4/4] sh_eth: Fix serialisation of interrupt disable
 with interrupt & NAPI handlers

On Thu, 2015-01-22 at 19:35 +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 01/22/2015 06:06 PM, Ben Hutchings wrote:
> 
> >>> In order to stop the RX path accessing the RX ring while it's being
> >>> stopped or resized, we clear the interrupt mask (EESIPR) and then call
> >>> free_irq() or synchronise_irq().  This is insufficient because the
> >>> interrupt handler or NAPI poller may set EESIPR again after we clear
> >>> it.
> 
> >>      Hm, how come the interrupt handler gets called when we have disabled all
> >> interrupts?
> 
> > It may be running on another processor and racing with the function that
> > clears EESIPR.
> 
>     Ah, I didn't think about SMP... but then we need more spinlock protection 
> instead, no?

That's what I tried first.  As we need to serialise with NAPI as well,
and napi_disable() may sleep, we need to call that first, so I ended up
with:

               napi_disable(&mdp->napi);
               spin_lock_irq(&mdp->lock);
               sh_eth_write(ndev, 0x0000, EESIPR);
               spin_unlock_irq(&mdp->lock);
               napi_enable(&mdp->napi);

But after napi_disable() sets the NAPI_STATE_DISABLE bit,
napi_schedule_prep() will return false and so the interrupt handler will
not clear the EESR_RX_CHECK bit any more.  This can leave the interrupt
screaming and prevent the NAPI handler from ever completing, so the
system is livelocked.

> >> Is it unmaskable EESR.ECI interrupt? BTW, I'm not seeing where the
> >> interrupt handler enables interrupts again; only NAPI poller does that AFAIK.
> 
> > Normally it only clears EESR_RX_CHECK, but as it cannot atomically clear
> > a single bit of EESIPR this can result in setting other bits.
> 
>     This is again only possible on SMP kernel, right?

Yes.

Ben.


--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html