linux-kernel - Issue with Broadcom wireless in 5.2rc1 (was Re: [PATCH] mmc: sdhci: queue work after sdhci_defer

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: <20190526122136.GA26456@basecamp>
Date:   Sun, 26 May 2019 08:21:36 -0400
From:   Brian Masney <masneyb@...tation.org>
To:     Adrian Hunter <adrian.hunter@...el.com>,
        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>,
        Wright Feng <wright.feng@...ress.com>
Cc:     ulf.hansson@...aro.org, faiz_abbas@...com,
        linux-mmc@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, Kalle Valo <kvalo@...eaurora.org>,
        linux-wireless@...r.kernel.org,
        brcm80211-dev-list.pdl@...adcom.com,
        brcm80211-dev-list@...ress.com, netdev@...r.kernel.org
Subject: Issue with Broadcom wireless in 5.2rc1 (was Re: [PATCH] mmc: sdhci:
 queue work after sdhci_defer_done())

+ Broadcom wireless maintainers

On Fri, May 24, 2019 at 11:49:58AM -0400, Brian Masney wrote:
> On Fri, May 24, 2019 at 03:17:13PM +0300, Adrian Hunter wrote:
> > On 24/05/19 2:10 PM, Brian Masney wrote:
> > > WiFi stopped working on the LG Nexus 5 phone and the issue was bisected
> > > to the commit c07a48c26519 ("mmc: sdhci: Remove finish_tasklet") that
> > > moved from using a tasklet to a work queue. That patch also changed
> > > sdhci_irq() to return IRQ_WAKE_THREAD instead of finishing the work when
> > > sdhci_defer_done() is true. Change it to queue work to the complete work
> > > queue if sdhci_defer_done() is true so that the functionality is
> > > equilivent to what was there when the finish_tasklet was present. This
> > > corrects the WiFi breakage on the Nexus 5 phone.
> > > 
> > > Signed-off-by: Brian Masney <masneyb@...tation.org>
> > > Fixes: c07a48c26519 ("mmc: sdhci: Remove finish_tasklet")
> > > ---
> > > [ ... ]
> > > 
> > >  drivers/mmc/host/sdhci.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c
> > > index 97158344b862..3563c3bc57c9 100644
> > > --- a/drivers/mmc/host/sdhci.c
> > > +++ b/drivers/mmc/host/sdhci.c
> > > @@ -3115,7 +3115,7 @@ static irqreturn_t sdhci_irq(int irq, void *dev_id)
> > >  			continue;
> > >  
> > >  		if (sdhci_defer_done(host, mrq)) {
> > > -			result = IRQ_WAKE_THREAD;
> > > +			queue_work(host->complete_wq, &host->complete_work);
> > 
> > The IRQ thread has a lot less latency than the work queue, which is why it
> > is done that way.
> > 
> > I am not sure why you say this change is equivalent to what was there
> > before, nor why it fixes your problem.
> > 
> > Can you explain some more?
>
> [ ... ]
> 
> drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c calls
> sdio_claim_host() and it appears to never return.

When the brcmfmac driver is loaded, the firmware is requested from disk,
and that's when the deadlock occurs in 5.2rc1. Specifically:

1) brcmf_sdio_download_firmware() in
   drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c calls
   sdio_claim_host()

2) brcmf_sdio_firmware_callback() is called and brcmf_sdiod_ramrw()
   tries to claim the host, but has to wait since its already claimed
   in #1 and the deadlock occurs.

I tried to release the host before the firmware is requested, however
parts of brcmf_chip_set_active() needs the host to be claimed, and a
similar deadlock occurs in brcmf_sdiod_ramrw() if I claim the host
before calling brcmf_chip_set_active().

I started to look at moving the sdio_{claim,release}_host() calls out of
brcmf_sdiod_ramrw() but there's a fair number of callers, so I'd like to
get feedback about the best course of action here.

Brian