2017-11-25 20:39:44

by Arend Van Spriel

[permalink] [raw]
Subject: [PATCH for-4.15] brcmfmac: change driver unbind order of the sdio function devices

In the function brcmf_sdio_firmware_callback() the driver is
unbound from the sdio function devices in the error path.
However, the order in which it is done resulted in a use-after-free
issue (see brcmf_ops_sdio_remove() in bcmsdh.c). Hence change
the order and first unbind sdio function #2 device and then
unbind sdio function #1 device.

Cc: [email protected] # v4.12.x
Fixes: 7a51461fc2da ("brcmfmac: unbind all devices upon failure in firmware callback")
Reported-by: Stefan Wahren <[email protected]>
Reviewed-by: Hante Meuleman <[email protected]>
Reviewed-by: Pieter-Paul Giesberts <[email protected]>
Reviewed-by: Franky Lin <[email protected]>
Signed-off-by: Arend van Spriel <[email protected]>
---
drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index e3495ea..3108f7ee 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -4121,8 +4121,8 @@ static void brcmf_sdio_firmware_callback(struct device *dev, int err,
sdio_release_host(sdiodev->func[1]);
fail:
brcmf_dbg(TRACE, "failed: dev=%s, err=%d\n", dev_name(dev), err);
- device_release_driver(dev);
device_release_driver(&sdiodev->func[2]->dev);
+ device_release_driver(dev);
}

struct brcmf_sdio *brcmf_sdio_probe(struct brcmf_sdio_dev *sdiodev)
--
1.9.1


2017-11-27 17:15:30

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH for-4.15] brcmfmac: change driver unbind order of the sdio function devices

Arend van Spriel <[email protected]> writes:

> In the function brcmf_sdio_firmware_callback() the driver is
> unbound from the sdio function devices in the error path.
> However, the order in which it is done resulted in a use-after-free
> issue (see brcmf_ops_sdio_remove() in bcmsdh.c). Hence change
> the order and first unbind sdio function #2 device and then
> unbind sdio function #1 device.
>
> Cc: [email protected] # v4.12.x
> Fixes: 7a51461fc2da ("brcmfmac: unbind all devices upon failure in firmware callback")
> Reported-by: Stefan Wahren <[email protected]>
> Reviewed-by: Hante Meuleman <[email protected]>
> Reviewed-by: Pieter-Paul Giesberts <[email protected]>
> Reviewed-by: Franky Lin <[email protected]>
> Signed-off-by: Arend van Spriel <[email protected]>

I'll queue this for v4.15.

--
Kalle Valo

2017-12-02 15:23:25

by Kalle Valo

[permalink] [raw]
Subject: Re: [for-4.15] brcmfmac: change driver unbind order of the sdio function devices

Arend Van Spriel <[email protected]> wrote:

> In the function brcmf_sdio_firmware_callback() the driver is
> unbound from the sdio function devices in the error path.
> However, the order in which it is done resulted in a use-after-free
> issue (see brcmf_ops_sdio_remove() in bcmsdh.c). Hence change
> the order and first unbind sdio function #2 device and then
> unbind sdio function #1 device.
>
> Cc: [email protected] # v4.12.x
> Fixes: 7a51461fc2da ("brcmfmac: unbind all devices upon failure in firmware callback")
> Reported-by: Stefan Wahren <[email protected]>
> Reviewed-by: Hante Meuleman <[email protected]>
> Reviewed-by: Pieter-Paul Giesberts <[email protected]>
> Reviewed-by: Franky Lin <[email protected]>
> Signed-off-by: Arend van Spriel <[email protected]>

Patch applied to wireless-drivers.git, thanks.

5c3de777bdaf brcmfmac: change driver unbind order of the sdio function devices

--
https://patchwork.kernel.org/patch/10075091/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches