* [PATCH] Fix kernel oops in failed chip_attach
@ 2016-05-04 19:30 Christian Daudt
2016-05-06 11:53 ` Kalle Valo
0 siblings, 1 reply; 4+ messages in thread
From: Christian Daudt @ 2016-05-04 19:30 UTC (permalink / raw)
To: linux-wireless
Cc: Christian Daudt, brcm80211-dev-list, Brett Rudley,
Arend van Spriel, Franky Lin, Hante Meuleman
When chip attach fails, brcmf_sdiod_intr_unregister is being called
but that is too early as sdiodev->settings has not been set yet
nor has brcmf_sdiod_intr_register been called.
Change to use oob_irq_requested + newly created sd_irq_requested
to decide on what to unregister at intr_unregister time.
Steps to reproduce problem:
- modprobe brcmfmac using buggy FW
- rmmod brcmfmac
- modprobe brcmfmac again.
If done with a buggy firmware, brcm_chip_attach will fail on the
2nd modprobe triggering the call to intr_unregister and the
kernel oops when attempting to de-reference sdiodev->settings->bus.sdio
which has not yet been set.
Signed-off-by: Christian Daudt <csd@broadcom.com>
---
.../wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 30 ++++++++++++----------
.../wireless/broadcom/brcm80211/brcmfmac/sdio.h | 1 +
2 files changed, 18 insertions(+), 13 deletions(-)
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index da0cdd3..09635a9 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -166,6 +166,7 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev)
sdio_claim_irq(sdiodev->func[1], brcmf_sdiod_ib_irqhandler);
sdio_claim_irq(sdiodev->func[2], brcmf_sdiod_dummy_irqhandler);
sdio_release_host(sdiodev->func[1]);
+ sdiodev->sd_irq_requested = true;
}
return 0;
@@ -173,27 +174,30 @@ int brcmf_sdiod_intr_register(struct brcmf_sdio_dev *sdiodev)
int brcmf_sdiod_intr_unregister(struct brcmf_sdio_dev *sdiodev)
{
- struct brcmfmac_sdio_pd *pdata;
- brcmf_dbg(SDIO, "Entering\n");
+ brcmf_dbg(SDIO, "Entering oob=%d sd=%d\n",
+ sdiodev->oob_irq_requested,
+ sdiodev->sd_irq_requested);
- pdata = &sdiodev->settings->bus.sdio;
- if (pdata->oob_irq_supported) {
+ if (sdiodev->oob_irq_requested) {
+ struct brcmfmac_sdio_pd *pdata;
+
+ pdata = &sdiodev->settings->bus.sdio;
sdio_claim_host(sdiodev->func[1]);
brcmf_sdiod_regwb(sdiodev, SDIO_CCCR_BRCM_SEPINT, 0, NULL);
brcmf_sdiod_regwb(sdiodev, SDIO_CCCR_IENx, 0, NULL);
sdio_release_host(sdiodev->func[1]);
- if (sdiodev->oob_irq_requested) {
- sdiodev->oob_irq_requested = false;
- if (sdiodev->irq_wake) {
- disable_irq_wake(pdata->oob_irq_nr);
- sdiodev->irq_wake = false;
- }
- free_irq(pdata->oob_irq_nr, &sdiodev->func[1]->dev);
- sdiodev->irq_en = false;
+ sdiodev->oob_irq_requested = false;
+ if (sdiodev->irq_wake) {
+ disable_irq_wake(pdata->oob_irq_nr);
+ sdiodev->irq_wake = false;
}
- } else {
+ free_irq(pdata->oob_irq_nr, &sdiodev->func[1]->dev);
+ sdiodev->irq_en = false;
+ }
+
+ if (sdiodev->sd_irq_requested) {
sdio_claim_host(sdiodev->func[1]);
sdio_release_irq(sdiodev->func[2]);
sdio_release_irq(sdiodev->func[1]);
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
index dcf0ce8c..c07ad25 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.h
@@ -186,6 +186,7 @@ struct brcmf_sdio_dev {
struct brcmf_bus *bus_if;
struct brcmf_mp_device *settings;
bool oob_irq_requested;
+ bool sd_irq_requested;
bool irq_en; /* irq enable flags */
spinlock_t irq_en_lock;
bool irq_wake; /* irq wake enable flags */
--
1.9.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix kernel oops in failed chip_attach
2016-05-04 19:30 [PATCH] Fix kernel oops in failed chip_attach Christian Daudt
@ 2016-05-06 11:53 ` Kalle Valo
2016-05-06 16:17 ` Christian Daudt
0 siblings, 1 reply; 4+ messages in thread
From: Kalle Valo @ 2016-05-06 11:53 UTC (permalink / raw)
To: Christian Daudt
Cc: linux-wireless, brcm80211-dev-list, Brett Rudley,
Arend van Spriel, Franky Lin, Hante Meuleman
Christian Daudt <csd@broadcom.com> writes:
> When chip attach fails, brcmf_sdiod_intr_unregister is being called
> but that is too early as sdiodev->settings has not been set yet
> nor has brcmf_sdiod_intr_register been called.
> Change to use oob_irq_requested + newly created sd_irq_requested
> to decide on what to unregister at intr_unregister time.
>
> Steps to reproduce problem:
> - modprobe brcmfmac using buggy FW
> - rmmod brcmfmac
> - modprobe brcmfmac again.
>
> If done with a buggy firmware, brcm_chip_attach will fail on the
> 2nd modprobe triggering the call to intr_unregister and the
> kernel oops when attempting to de-reference sdiodev->settings->bus.sdio
> which has not yet been set.
>
> Signed-off-by: Christian Daudt <csd@broadcom.com>
Please use prefix "brcmfmac: " in the title.
--
Kalle Valo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix kernel oops in failed chip_attach
2016-05-06 11:53 ` Kalle Valo
@ 2016-05-06 16:17 ` Christian Daudt
2016-05-06 16:29 ` Kalle Valo
0 siblings, 1 reply; 4+ messages in thread
From: Christian Daudt @ 2016-05-06 16:17 UTC (permalink / raw)
To: Kalle Valo
Cc: linux-wireless, brcm80211-dev-list, Brett Rudley,
Arend van Spriel, Franky Lin, Hante Meuleman
On Fri, May 6, 2016 at 4:53 AM, Kalle Valo <kvalo@codeaurora.org> wrote:
> Christian Daudt <csd@broadcom.com> writes:
>
>> When chip attach fails, brcmf_sdiod_intr_unregister is being called
>> but that is too early as sdiodev->settings has not been set yet
>> nor has brcmf_sdiod_intr_register been called.
>> Change to use oob_irq_requested + newly created sd_irq_requested
>> to decide on what to unregister at intr_unregister time.
>>
>> Steps to reproduce problem:
>> - modprobe brcmfmac using buggy FW
>> - rmmod brcmfmac
>> - modprobe brcmfmac again.
>>
>> If done with a buggy firmware, brcm_chip_attach will fail on the
>> 2nd modprobe triggering the call to intr_unregister and the
>> kernel oops when attempting to de-reference sdiodev->settings->bus.sdio
>> which has not yet been set.
>>
>> Signed-off-by: Christian Daudt <csd@broadcom.com>
>
> Please use prefix "brcmfmac: " in the title.
>
I'll resubmit with that mod after getting round 1 feedback. I was a
bit rusty on patch submissions and forgot about that part on the first
one.
Thanks,
csd
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] Fix kernel oops in failed chip_attach
2016-05-06 16:17 ` Christian Daudt
@ 2016-05-06 16:29 ` Kalle Valo
0 siblings, 0 replies; 4+ messages in thread
From: Kalle Valo @ 2016-05-06 16:29 UTC (permalink / raw)
To: Christian Daudt
Cc: linux-wireless, brcm80211-dev-list, Brett Rudley,
Arend van Spriel, Franky Lin, Hante Meuleman
Christian Daudt <csd@broadcom.com> writes:
> On Fri, May 6, 2016 at 4:53 AM, Kalle Valo <kvalo@codeaurora.org> wrote:
>> Christian Daudt <csd@broadcom.com> writes:
>>
>>> When chip attach fails, brcmf_sdiod_intr_unregister is being called
>>> but that is too early as sdiodev->settings has not been set yet
>>> nor has brcmf_sdiod_intr_register been called.
>>> Change to use oob_irq_requested + newly created sd_irq_requested
>>> to decide on what to unregister at intr_unregister time.
>>>
>>> Steps to reproduce problem:
>>> - modprobe brcmfmac using buggy FW
>>> - rmmod brcmfmac
>>> - modprobe brcmfmac again.
>>>
>>> If done with a buggy firmware, brcm_chip_attach will fail on the
>>> 2nd modprobe triggering the call to intr_unregister and the
>>> kernel oops when attempting to de-reference sdiodev->settings->bus.sdio
>>> which has not yet been set.
>>>
>>> Signed-off-by: Christian Daudt <csd@broadcom.com>
>>
>> Please use prefix "brcmfmac: " in the title.
>>
> I'll resubmit with that mod after getting round 1 feedback. I was a
> bit rusty on patch submissions and forgot about that part on the first
> one.
No worries, that's why we do review :)
--
Kalle Valo
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-05-06 16:29 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-04 19:30 [PATCH] Fix kernel oops in failed chip_attach Christian Daudt
2016-05-06 11:53 ` Kalle Valo
2016-05-06 16:17 ` Christian Daudt
2016-05-06 16:29 ` Kalle Valo
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).