netdev - Re: [PATCH] brcmfmac: abort and release host after error

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: <f9f1dfab-2c48-f37b-836b-6dc7fa5bc801@roeck-us.net>
Date:   Tue, 28 Jan 2020 20:17:39 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     Dan Carpenter <dan.carpenter@...cle.com>
Cc:     Kalle Valo <kvalo@...eaurora.org>,
        Arend van Spriel <arend.vanspriel@...adcom.com>,
        Franky Lin <franky.lin@...adcom.com>,
        Hante Meuleman <hante.meuleman@...adcom.com>,
        Chi-Hsien Lin <chi-hsien.lin@...ress.com>,
        linux-wireless@...r.kernel.org,
        brcm80211-dev-list.pdl@...adcom.com,
        brcm80211-dev-list@...ress.com, netdev@...r.kernel.org,
        linux-kernel@...r.kernel.org, Matthias Kaehlcke <mka@...omium.org>,
        Brian Norris <briannorris@...omium.org>,
        Douglas Anderson <dianders@...omium.org>
Subject: Re: [PATCH] brcmfmac: abort and release host after error

On 1/28/20 7:32 PM, Dan Carpenter wrote:
> On Tue, Jan 28, 2020 at 02:14:57PM -0800, Guenter Roeck wrote:
>> With commit 216b44000ada ("brcmfmac: Fix use after free in
>> brcmf_sdio_readframes()") applied, we see locking timeouts in
>> brcmf_sdio_watchdog_thread().
>>
>> brcmfmac: brcmf_escan_timeout: timer expired
>> INFO: task brcmf_wdog/mmc1:621 blocked for more than 120 seconds.
>> Not tainted 4.19.94-07984-g24ff99a0f713 #1
>> "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
>> brcmf_wdog/mmc1 D    0   621      2 0x00000000 last_sleep: 2440793077.  last_runnable: 2440766827
>> [<c0aa1e60>] (__schedule) from [<c0aa2100>] (schedule+0x98/0xc4)
>> [<c0aa2100>] (schedule) from [<c0853830>] (__mmc_claim_host+0x154/0x274)
>> [<c0853830>] (__mmc_claim_host) from [<bf10c5b8>] (brcmf_sdio_watchdog_thread+0x1b0/0x1f8 [brcmfmac])
>> [<bf10c5b8>] (brcmf_sdio_watchdog_thread [brcmfmac]) from [<c02570b8>] (kthread+0x178/0x180)
>>
>> In addition to restarting or exiting the loop, it is also necessary to
>> abort the command and to release the host.
>>
>> Fixes: 216b44000ada ("brcmfmac: Fix use after free in brcmf_sdio_readframes()")
> 
> Huh...  Thanks for fixing the bug.  That seems to indicate that we were
> triggering the use after free but no one noticed at runtime.  With

Actually, we did see the problem. We just didn't realize it.

> kfree(), a use after free can be harmless if you don't have poisoning
> enabled and no other thread has re-used the memory.  I'm not sure about
> kfree_skb() but presumably it's the same.
> 

Not really; it ultimately does result in a crash. We see that in ChromeOS
R80 (and probably in all earlier releases, but I didn't check), which does
not (yet) include 216b44000ada. The upcoming R81, which does include
216b44000ada, doesn't crash but there are lots of stalls like the one
above. The combination of both (ie the difference in behavior) helped
tracking down the problem.

Guenter