summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDanny van Heumen <danny@dannyvanheumen.nl>2022-07-11 23:21:16 +0000
committerKalle Valo <kvalo@kernel.org>2022-07-28 12:59:05 +0300
commitcb774bd35318c1b4cb61f6f2caac85537d07fbde (patch)
tree5421531a77c2a70fa03e4255ec1b6b2138c76475
parent5c54ab24377b999c7f1c30b41218e6490cd4ac80 (diff)
wifi: brcmfmac: prevent double-free on hardware-reset
In case of buggy firmware, brcmfmac may perform a hardware reset. If during reset and subsequent probing an early failure occurs, a memory region is accidentally double-freed. With hardened memory allocation enabled, this error will be detected. - return early where appropriate to skip unnecessary clean-up. - set '.freezer' pointer to NULL to prevent double-freeing under possible other circumstances and to re-align result under various different behaviors of memory allocation freeing. - correctly claim host on func1 for disabling func2. - after reset, do not initiate probing immediately, but rely on events. Given a firmware crash, function 'brcmf_sdio_bus_reset' is called. It calls 'brcmf_sdiod_remove', then follows up with 'brcmf_sdiod_probe' to reinitialize the hardware. If 'brcmf_sdiod_probe' fails to "set F1 blocksize", it exits early, which includes calling 'brcmf_sdiod_remove'. In both cases 'brcmf_sdiod_freezer_detach' is called to free allocated '.freezer', which has not yet been re-allocated the second time. Stacktrace of (failing) hardware reset after firmware-crash: Code: b9402b82 8b0202c0 eb1a02df 54000041 (d4210000) ret_from_fork+0x10/0x20 kthread+0x154/0x160 worker_thread+0x188/0x504 process_one_work+0x1f4/0x490 brcmf_core_bus_reset+0x34/0x44 [brcmfmac] brcmf_sdio_bus_reset+0x68/0xc0 [brcmfmac] brcmf_sdiod_probe+0x170/0x21c [brcmfmac] brcmf_sdiod_remove+0x48/0xc0 [brcmfmac] kfree+0x210/0x220 __slab_free+0x58/0x40c Call trace: x2 : 0000000000000040 x1 : fffffc00002d2b80 x0 : ffff00000b4aee40 x5 : ffff8000013fa728 x4 : 0000000000000001 x3 : ffff00000b4aee00 x8 : ffff800009967ce0 x7 : ffff8000099bfce0 x6 : 00000006f8005d01 x11: ffff8000099bfce0 x10: 00000000fffff000 x9 : ffff8000083401d0 x14: 0000000000000000 x13: 657a69736b636f6c x12: 6220314620746573 x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000030 x20: fffffc00002d2ba0 x19: fffffc00002d2b80 x18: 0000000000000000 x23: ffff00000b4aee00 x22: ffff00000b4aee00 x21: 0000000000000001 x26: ffff00000b4aee00 x25: ffff0000f7753705 x24: 000000000001288a x29: ffff80000a22bbf0 x28: ffff000000401200 x27: 000000008020001a sp : ffff80000a22bbf0 lr : kfree+0x210/0x220 pc : __slab_free+0x58/0x40c pstate: 60000005 (nZCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--) Workqueue: events brcmf_core_bus_reset [brcmfmac] Hardware name: Pine64 Pinebook Pro (DT) CPU: 2 PID: 639 Comm: kworker/2:2 Tainted: G C 5.16.0-0.bpo.4-arm64 #1 Debian 5.16.12-1~bpo11+1 nvmem_rockchip_efuse industrialio_triggered_buffer videodev snd_soc_core snd_pcm_dmaengine kfifo_buf snd_pcm io_domain mc industrialio mt> Modules linked in: snd_seq_dummy snd_hrtimer snd_seq snd_seq_device nft_fib_inet nft_fib_ipv4 nft_fib_ipv6 nft_fib nft_reject_inet nf_reje> Internal error: Oops - BUG: 0 [#1] SMP kernel BUG at mm/slub.c:379! Signed-off-by: Danny van Heumen <danny@dannyvanheumen.nl> Reviewed-by: Arend van Spriel <aspriel.gmail.com> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Signed-off-by: Kalle Valo <kvalo@kernel.org> Link: https://lore.kernel.org/r/id1HN6qCMAirApBzTA6fT7ZFWBBGCJhULpflxQ7NT6cgCboVnn3RHpiOFjA9SbRqzBRFLk9ES0C4FNvO6fUQsNg7pqF6ZSNAYUo99nHy8PY=@dannyvanheumen.nl
-rw-r--r--drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c13
-rw-r--r--drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c10
2 files changed, 6 insertions, 17 deletions
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index 62d5c5f5fbc8..d639bb8b51ae 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
@@ -804,6 +804,7 @@ static void brcmf_sdiod_freezer_detach(struct brcmf_sdio_dev *sdiodev)
if (sdiodev->freezer) {
WARN_ON(atomic_read(&sdiodev->freezer->freezing));
kfree(sdiodev->freezer);
+ sdiodev->freezer = NULL;
}
}
@@ -870,13 +871,9 @@ int brcmf_sdiod_remove(struct brcmf_sdio_dev *sdiodev)
brcmf_sdiod_freezer_detach(sdiodev);
- /* Disable Function 2 */
- sdio_claim_host(sdiodev->func2);
- sdio_disable_func(sdiodev->func2);
- sdio_release_host(sdiodev->func2);
-
- /* Disable Function 1 */
+ /* Disable functions 2 then 1. */
sdio_claim_host(sdiodev->func1);
+ sdio_disable_func(sdiodev->func2);
sdio_disable_func(sdiodev->func1);
sdio_release_host(sdiodev->func1);
@@ -906,7 +903,7 @@ int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev)
if (ret) {
brcmf_err("Failed to set F1 blocksize\n");
sdio_release_host(sdiodev->func1);
- goto out;
+ return ret;
}
switch (sdiodev->func2->device) {
case SDIO_DEVICE_ID_BROADCOM_CYPRESS_4373:
@@ -928,7 +925,7 @@ int brcmf_sdiod_probe(struct brcmf_sdio_dev *sdiodev)
if (ret) {
brcmf_err("Failed to set F2 blocksize\n");
sdio_release_host(sdiodev->func1);
- goto out;
+ return ret;
} else {
brcmf_dbg(SDIO, "set F2 blocksize to %d\n", f2_blksz);
}
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
index 762e887e8ea4..8968809399c7 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/sdio.c
@@ -4151,7 +4151,6 @@ int brcmf_sdio_get_fwname(struct device *dev, const char *ext, u8 *fw_name)
static int brcmf_sdio_bus_reset(struct device *dev)
{
- int ret = 0;
struct brcmf_bus *bus_if = dev_get_drvdata(dev);
struct brcmf_sdio_dev *sdiodev = bus_if->bus_priv.sdio;
@@ -4168,14 +4167,7 @@ static int brcmf_sdio_bus_reset(struct device *dev)
sdio_release_host(sdiodev->func1);
brcmf_bus_change_state(sdiodev->bus_if, BRCMF_BUS_DOWN);
-
- ret = brcmf_sdiod_probe(sdiodev);
- if (ret) {
- brcmf_err("Failed to probe after sdio device reset: ret %d\n",
- ret);
- }
-
- return ret;
+ return 0;
}
static const struct brcmf_bus_ops brcmf_sdio_bus_ops = {