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

Am 06.10.25 um 19:08 schrieb Richard Reigh:
> On Mon, Oct 6, 2025 at 10:47 AM Stefan Wahren <[email protected]> wrote:
>> 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
> Stefan,
>
> Yes, I plan on submitting a V2 patch, after I finish testing the change.
>
> I will also add the suggested fixes tag to the patch submission.
Sure, take your time.

Thanks
>
> Thanks,
> Richard