Re: [PATCH v5] wifi: brcmfmac: Fix use-after-free bug in brcmf_cfg80211_detach

[Arend Van Spriel <arend.vanspriel@broadcom.com>]

[-- Attachment #1: Type: text/plain, Size: 3119 bytes --]

On November 6, 2023 3:44:53 PM Zheng Hacker <hackerzheng666@gmail.com> wrote:

> Thanks! I didn't test it for I don't have a device. Very appreciated
> if anyone could help with that.

I would volunteer, but it made me dig deep and not sure if there is a 
problem to solve here.

brcmf_cfg80211_detach() calls wl_deinit_priv() -> brcmf_abort_scanning() -> 
brcmf_notify_escan_complete() which does delete the timer.

What am I missing here?

Regards,
Arend

>
> Kalle Valo <kvalo@kernel.org> 于2023年11月6日周一 22:41写道：
>>
>> Zheng Wang <zyytlz.wz@163.com> writes:
>>
>>> This is the candidate patch of CVE-2023-47233 :
>>> https://nvd.nist.gov/vuln/detail/CVE-2023-47233
>>>
>>> In brcm80211 driver,it starts with the following invoking chain
>>> to start init a timeout worker:
>>>
>>> ->brcmf_usb_probe
>>> ->brcmf_usb_probe_cb
>>> ->brcmf_attach
>>> ->brcmf_bus_started
>>>  ->brcmf_cfg80211_attach
>>>    ->wl_init_priv
>>>      ->brcmf_init_escan
>>>        ->INIT_WORK(&cfg->escan_timeout_work,
>>>          brcmf_cfg80211_escan_timeout_worker);
>>>
>>> If we disconnect the USB by hotplug, it will call
>>> brcmf_usb_disconnect to make cleanup. The invoking chain is :
>>>
>>> brcmf_usb_disconnect
>>> ->brcmf_usb_disconnect_cb
>>> ->brcmf_detach
>>> ->brcmf_cfg80211_detach
>>>  ->kfree(cfg);
>>>
>>> While the timeout woker may still be running. This will cause
>>> a use-after-free bug on cfg in brcmf_cfg80211_escan_timeout_worker.
>>>
>>> Fix it by deleting the timer and canceling the worker in
>>> brcmf_cfg80211_detach.
>>>
>>> Fixes: e756af5b30b0 ("brcmfmac: add e-scan support.")
>>> Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
>>> Cc: stable@vger.kernel.org
>>> ---
>>> v5:
>>> - replace del_timer_sync with timer_shutdown_sync suggested by
>>> Arend and Takashi
>>> v4:
>>> - rename the subject and add CVE number as Ping-Ke Shih suggested
>>> v3:
>>> - rename the subject as Johannes suggested
>>> v2:
>>> - fix the error of kernel test bot reported
>>> ---
>>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c | 2 ++
>>> 1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c 
>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> index 667462369a32..a8723a61c9e4 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
>>> @@ -8431,6 +8431,8 @@ void brcmf_cfg80211_detach(struct brcmf_cfg80211_info 
>>> *cfg)
>>> if (!cfg)
>>>        return;
>>>
>>> +     timer_shutdown_sync(&cfg->escan_timeout);
>>> +     cancel_work_sync(&cfg->escan_timeout_work);
>>> brcmf_pno_detach(cfg);
>>> brcmf_btcoex_detach(cfg);
>>> wiphy_unregister(cfg->wiphy);
>>
>> Has anyone tested this on a real device? As v1 didn't even compile I am
>> very cautious:
>>
>> https://patchwork.kernel.org/project/linux-wireless/patch/20231104054709.716585-1-zyytlz.wz@163.com/
>>
>> --
>> https://patchwork.kernel.org/project/linux-wireless/list/
>>
>> https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches




[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4219 bytes --]