From patchwork Tue May 24 16:51:45 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Danny van Heumen X-Patchwork-Id: 576819 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 0CA45C433F5 for ; Tue, 24 May 2022 16:52:04 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S238066AbiEXQwD (ORCPT ); Tue, 24 May 2022 12:52:03 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:33104 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234234AbiEXQv7 (ORCPT ); Tue, 24 May 2022 12:51:59 -0400 Received: from mail-0201.mail-europe.com (mail-0201.mail-europe.com [51.77.79.158]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 467033B54E for ; Tue, 24 May 2022 09:51:57 -0700 (PDT) Date: Tue, 24 May 2022 16:51:45 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=dannyvanheumen.nl; s=protonmail2; t=1653411112; x=1653670312; bh=eub/et+mHH8VEHCirBk8CEjJyfwhvFaMCP9BnHpivR4=; h=Date:To:From:Reply-To:Subject:Message-ID:Feedback-ID:From:To:Cc: Date:Subject:Reply-To:Feedback-ID:Message-ID; b=f1qHqhS4rH/7CTLRQqhKUdNnGFDc+FfoylppBrQCBZqGNjN1haXmUtCfkF+0QrMEP rMYxwgUYDLUW+KriohCNPE2MO8ZD+LUJDzKC8bO9RIf0F1upHWKCaLIr67tZTkYGjA KtN/3Aiq1Dy79VmPq7ijTbZmKAFOb70CD2JiIrA5EAHFZH8qhX6drOJ2q7eWfC5gNF uD7GEu8MZb1KMJegSL294XB+Zp9s6/lf9OsnR8adJdIrreALigyWMf6jz8coAOf7wh gUM2nuP4cUpeqn9hapUdebnxY0idA9bkzWgm68tWXKBvujEiI47ygI3DPhpQVozlLa K5LF0r1WYy0MA== To: Arend van Spriel , Franky Lin , Hante Meuleman , "linux-wireless@vger.kernel.org" , "brcm80211-dev-list.pdl@broadcom.com" , "SHA-cyfmac-dev-list@infineon.com" From: Danny van Heumen Reply-To: Danny van Heumen Subject: [PATCH] work-in-progress: double-free after hardware reset due to firmware-crash Message-ID: Feedback-ID: 15073070:user:proton MIME-Version: 1.0 Precedence: bulk List-ID: X-Mailing-List: linux-wireless@vger.kernel.org Dear all, I am not a regular C developer nor kernel developer. I don't regularly report issues, so I will probably do things wrong. I investigated a crash that IIUC occurs with hardened memory allocation enabled and a firmware-crash followed by an early failure during hardware reinitialization/probing. The hardened allocator detects double-free issue. I have created the patch (see attachment) against linux-5.18. Though, please check carefully, because I have not been able to confirm that it actually works. I am hoping someone familiar with the code-base can either test this easily, or confirm from review/analysis. The commit message describes it in more detail. In summary: 'brcmf_sdio_bus_reset' cleans up and reinitializes the hardware. It frees memory used by (struct brcmf_sdio_dev)->freezer (struct brcmf_sdiod_freezer). However, it then goes to probe the hardware, and an early failure to probe results in the same freeing, both called through function 'brcmf_sdiod_freezer_detach' called inside 'brcmf_sdiod_remove'. Which results in double freeing. As mentioned before, I was not able to test this and I do not regularly develop in C. I am not confident that this is the proper way to fix it, but it seemed obvious enough. I hope you can support in fixing this bug. Kind regards, Danny PS: Please let me know if I am doing things wrong. I have included both maintainers and mailing lists from https://docs.kernel.org/process/maintainers.html#broadcom-brcm80211-ieee802-11n-wireless-driver I hope I this is alright. >From ddb281e2ebf7e6bd89f091403616a9d77d73be63 Mon Sep 17 00:00:00 2001 From: Danny van Heumen Date: Tue, 24 May 2022 18:30:50 +0200 Subject: [PATCH] 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. 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! --- drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c index ac02244a6fdf..70a664f2a697 100644 --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c @@ -802,6 +802,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; } } -- 2.34.1