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

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.

Thanks,
Richard