netdev - Re: stmmac: Race in coalesce timer and NAPI

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]

Date:   Fri, 21 Sep 2018 06:54:10 -0700
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Jose Abreu <Jose.Abreu@...opsys.com>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Joao Pinto <Joao.Pinto@...opsys.com>
Subject: Re: stmmac: Race in coalesce timer and NAPI



On 09/21/2018 02:19 AM, Jose Abreu wrote:
> Hello,
> 
> I'm getting a race in stmmac coalesce timer and the
> napi_schedule() interrupt and I'm asking for advice. Currently,
> we are scheduling NAPI in coalesce timer but this leads to
> stmmac_tx_clean() deadlock because this function tries to acquire
> queue lock.

This is strange. Which lock are you talking about ?

The napi_schedule() stuff should be enough to protect your use case.


> 
> I find that this is not expected because only one instance of
> NAPI should run at same time so I was wondering if it is possible
> that xmit() callback is causing the deadlock ?
> 
> BTW, this is solved by:
>     - Directly call stmmac_tx_clean() in timer function AND
>     - Use netif_tx_trylock() in stmmac_tx_clean(). Then, if queue
> is already locked we re-arm coalesce timer or reschedule NAPI.
> 
> This is easily reproducible in an ARM board with 8 core running
> at 100MHz each.
> 
> Thanks and Best Regards,
> Jose Miguel Abreu
> 

It looks to me stmmac_napi_poll() should not apply/consume any budget for TX completion.

The budget for a NAPI poll shared by RX and TX is really only for the RX side.

netpoll will specificall call the poll() with budget==0 to only drain TX