netdev - Re: Re: Commit 05cf0d1bf4 ("net: stmmac: free an skb first when there are no longer any descriptors using it") breaks stmmac?

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]

Message-ID: <d102d99b-3fa2-14a9-f1bb-15826c74e26c@axis.com>
Date:   Tue, 13 Feb 2018 14:33:44 +0100
From:   Niklas Cassel <niklas.cassel@...s.com>
To:     Niklas Cassel <niklass@...s.com>,
        Jose Abreu <Jose.Abreu@...opsys.com>
CC:     Joao Pinto <Joao.Pinto@...opsys.com>,
        linux-netdev <netdev@...r.kernel.org>,
        Giuseppe CAVALLARO <peppe.cavallaro@...com>,
        <alexandre.torgue@...com>
Subject: Re: Re: Commit 05cf0d1bf4 ("net: stmmac: free an skb first when there
 are no longer any descriptors using it") breaks stmmac?

Hello Jose,

I remember that you had a problem
with a use after free in stmmac_tx_clean().
I still don't think that it is related to
commit 05cf0d1bf4, however, when comparing
the stmmac driver to the amd-xgbe driver
I realized that:

xgbe_tx_poll() has both a smp_rmb() after fetching
cur_tx, and also a dma_rmb() after reading the own
bit, before reading any other descriptor fields.

stmmac_tx_clean() has neither a smp_rmb() or a
dma_rmb().

Also
xgbe_dev_xmit() has a dma_wmb() _before_ setting
the own bit, and a smp_wmb() after setting the own
bit.

stmmac simply has a dma_wmb() _after_ setting the
own bit.

I assume you are using a SMP system.

If you can still reproduce your problem quite easily,
perhaps you could try to make stmmac look more like
xgbe in these regards, and see if that solves your
use after free crash in stmmac_tx_clean().

Kind regards,
Niklas