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

Hi Richard,

Am 04.10.25 um 11:22 schrieb Arend van Spriel:
> 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.
thanks. @Richard Do you plan to submit a V2 patch?

Best regards
>
> Regards,
> Arend