Re: [PATCH] brcmfmac: Fix NULL pointer dereference in P2P action frame handling

On 10/3/2025 7:02 PM, Stefan Wahren wrote:
> Hi,
> 
> Am 03.10.25 um 18:00 schrieb Arend van Spriel:
>> Op 3 oktober 2025 01:20:21 schreef Richard Reigh <[email protected]>:
>>
>>> When iPhones query network information via the WiFi settings "i" button,
>>> the brcmfmac driver crashes with a NULL pointer dereference. This occurs
>>> because the P2P device vif is not initialized when handling certain 
>>> action
>>> frames.
>>
>> That does make a lot more sense than the line that gdb gave me with my 
>> driver rebuild attempt.
>>
>>> Add NULL check for vif before dereferencing in 
>>> brcmf_p2p_tx_action_frame()
>>> and return -ENODEV when vif is NULL. Also modify
>>
>> Now I don't think it is necessary to bail out here. See my suggestion 
>> below...
>>
>>> the retry loop in
>>> brcmf_p2p_send_action_frame() to stop immediately on permanent failures
>>> rather than retrying.
>>>
>>> Tested on Raspberry Pi Zero 2 W with kernel 6.1.21-v8+
>>>
>>> Signed-off-by: Richard Reigh <[email protected]>
>>> ---
>>> .../wireless/broadcom/brcm80211/brcmfmac/p2p.c   | 16 +++++++++++++++-
>>> 1 file changed, 15 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c 
>>> b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
>>> index 10d9d9c63..2c73156fa 100644
>>> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
>>> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
>>> @@ -1559,6 +1559,12 @@ static s32 brcmf_p2p_tx_action_frame(struct 
>>> brcmf_p2p_info *p2p,
>>> else
>>> vif = p2p->bss_idx[P2PAPI_BSSCFG_DEVICE].vif;
>>>
>>> + /* add NULL check */
>>> + if (!vif) {
>>> + brcmf_err("vif is NULL, cannot send action frame\n");
>>> + return -ENODEV;
>>> + }
>>
>> Instead of giving up and surrender we can use primary vif, ie. p2p- 
>> >bss_idx[P2PAPI_BSSCFG_PRIMARY].vif:
>>
>> if (!vif) {
>>        brcmf_dbg(TRACE, "no P2P device setup\n");
>>        vif = p2p->bss_idx[P2PAPI_BSSCFG_PRIMARY].vif;
>> }
> this looks much better. But the whole patch needs a fixes tag.

Sure.

> In case this issue is reproducible before Linux 5.8, the tag should 
> reference:
> 18e2f61db3b7 ("brcmfmac: P2P action frame tx.")
> 
> otherwise it should reference:
> d524d5ce3655 ("brcmfmac: p2p cert 6.1.9-support GOUT handling p2p 
> presence request")

Both good suggestions. Looking at the code I would say that 18e2f61db3b7 
("brcmfmac: P2P action frame tx.") is the best choice.

Regards,
Arend