Hi Danny,
>>
>> ------- Original Message -------
>> On Tuesday, May 24th, 2022 at 18:51, Danny van Heumen danny@xxxxxxxxxxxxxxxxx wrote:
>>>
>>> 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.
>>>
Thanks for reporting and sending out a patch to fix this.
If the problem is double freeing the freezer buffer, it should be addressed from the root by setting pointer to NULL. Same thing might need to be done for sg table as well. Sorry I don’t have any sdio module to reproduce and test. Please see if the below change fixes the problem.
diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/bcmsdh.c
index ac02244a6fdf..e9bad7197ba9 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;
}
}
@@ -885,7 +886,11 @@ int brcmf_sdiod_remove(struct brcmf_sdio_dev *sdiodev)
sdio_disable_func(sdiodev->func1);
sdio_release_host(sdiodev->func1);
- sg_free_table(&sdiodev->sgtable);
+ if (sdiodev->sgtable) {
+ sg_free_table(&sdiodev->sgtable);
+ sdiodev->sgtable = NULL;
+ }
+
sdiodev->sbwad = 0;
pm_runtime_allow(sdiodev->func1->card->host->parent);
As for submitting patch to linux-wireless, please follow the guideline. [1]
Thanks,
- Franky
[1] https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
--
This electronic communication and the information and any files transmitted
with it, or attached to it, are confidential and are intended solely for
the use of the individual or entity to whom it is addressed and may contain
information that is confidential, legally privileged, protected by privacy
laws, or otherwise restricted from disclosure to anyone else. If you are
not the intended recipient or the person responsible for delivering the
e-mail to the intended recipient, you are hereby notified that any use,
copying, distributing, dissemination, forwarding, printing, or copying of
this e-mail is strictly prohibited. If you received this e-mail in error,
please return the e-mail to the sender, delete it from your computer, and
destroy any printed copy of it.
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature
