linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Guenter Roeck <linux@roeck-us.net>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Kalle Valo <kvalo@codeaurora.org>,
	Arend van Spriel <arend.vanspriel@broadcom.com>,
	Franky Lin <franky.lin@broadcom.com>,
	Hante Meuleman <hante.meuleman@broadcom.com>,
	Chi-Hsien Lin <chi-hsien.lin@cypress.com>,
	linux-wireless@vger.kernel.org,
	brcm80211-dev-list.pdl@broadcom.com,
	brcm80211-dev-list@cypress.com, netdev@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	Matthias Kaehlcke <mka@chromium.org>,
	Brian Norris <briannorris@chromium.org>,
	Douglas Anderson <dianders@chromium.org>
Subject: Re: [PATCH] brcmfmac: abort and release host after error
Date: Tue, 28 Jan 2020 20:17:39 -0800	[thread overview]
Message-ID: <f9f1dfab-2c48-f37b-836b-6dc7fa5bc801@roeck-us.net> (raw)
In-Reply-To: <20200129033257.GC1754@kadam>

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

  reply	other threads:[~2020-01-29  4:17 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-28 22:14 [PATCH] brcmfmac: abort and release host after error Guenter Roeck
2020-01-28 23:14 ` Doug Anderson
2020-01-29  0:05   ` Guenter Roeck
2020-01-29  0:57     ` Franky Lin
2020-01-29 18:04       ` Brian Norris
2020-01-29 23:12         ` Franky Lin
2020-01-29  3:32 ` Dan Carpenter
2020-01-29  4:17   ` Guenter Roeck [this message]
2020-02-12 16:18 ` Kalle Valo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=f9f1dfab-2c48-f37b-836b-6dc7fa5bc801@roeck-us.net \
    --to=linux@roeck-us.net \
    --cc=arend.vanspriel@broadcom.com \
    --cc=brcm80211-dev-list.pdl@broadcom.com \
    --cc=brcm80211-dev-list@cypress.com \
    --cc=briannorris@chromium.org \
    --cc=chi-hsien.lin@cypress.com \
    --cc=dan.carpenter@oracle.com \
    --cc=dianders@chromium.org \
    --cc=franky.lin@broadcom.com \
    --cc=hante.meuleman@broadcom.com \
    --cc=kvalo@codeaurora.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-wireless@vger.kernel.org \
    --cc=mka@chromium.org \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).