Re: [PATCH wireless-next] wifi: brcmfmac: fix crash while sending Action Frames in standalone AP Mode

On 10/9/2025 9:39 AM, Gokul Sivakumar wrote:
> Currently, whenever there is a need to transmit an Action frame,
> the brcmfmac driver always uses the P2P vif to send the "actframe" IOVAR to
> firmware. The P2P interfaces were available when wpa_supplicant is managing
> the wlan interface.
> 
> However, the P2P interfaces are not created/initialized when only hostapd
> is managing the wlan interface. And if hostapd receives an ANQP Query REQ
> Action frame even from an un-associated STA, the brcmfmac driver tries
> to use an uninitialized P2P vif pointer for sending the IOVAR to firmware.
> This NULL pointer dereferencing triggers a driver crash.
> 
>   [ 1417.074538] Unable to handle kernel NULL pointer dereference at virtual
>   address 0000000000000000
>   [...]
>   [ 1417.075188] Hardware name: Raspberry Pi 4 Model B Rev 1.5 (DT)
>   [...]
>   [ 1417.075653] Call trace:
>   [ 1417.075662]  brcmf_p2p_send_action_frame+0x23c/0xc58 [brcmfmac]
>   [ 1417.075738]  brcmf_cfg80211_mgmt_tx+0x304/0x5c0 [brcmfmac]
>   [ 1417.075810]  cfg80211_mlme_mgmt_tx+0x1b0/0x428 [cfg80211]
>   [ 1417.076067]  nl80211_tx_mgmt+0x238/0x388 [cfg80211]
>   [ 1417.076281]  genl_family_rcv_msg_doit+0xe0/0x158
>   [ 1417.076302]  genl_rcv_msg+0x220/0x2a0
>   [ 1417.076317]  netlink_rcv_skb+0x68/0x140
>   [ 1417.076330]  genl_rcv+0x40/0x60
>   [ 1417.076343]  netlink_unicast+0x330/0x3b8
>   [ 1417.076357]  netlink_sendmsg+0x19c/0x3f8
>   [ 1417.076370]  __sock_sendmsg+0x64/0xc0
>   [ 1417.076391]  ____sys_sendmsg+0x268/0x2a0
>   [ 1417.076408]  ___sys_sendmsg+0xb8/0x118
>   [ 1417.076427]  __sys_sendmsg+0x90/0xf8
>   [ 1417.076445]  __arm64_sys_sendmsg+0x2c/0x40
>   [ 1417.076465]  invoke_syscall+0x50/0x120
>   [ 1417.076486]  el0_svc_common.constprop.0+0x48/0xf0
>   [ 1417.076506]  do_el0_svc+0x24/0x38
>   [ 1417.076525]  el0_svc+0x30/0x100
>   [ 1417.076548]  el0t_64_sync_handler+0x100/0x130
>   [ 1417.076569]  el0t_64_sync+0x190/0x198
>   [ 1417.076589] Code: f9401e80 aa1603e2 f9403be1 5280e483 (f9400000)
> 
> Fix this, by always using the vif corresponding to the wdev on which the
> Action frame Transmission request was initiated by the userspace. This way,
> even if P2P vif is not available, the IOVAR is sent to firmware on AP vif
> and the ANQP Query RESP Action frame is transmitted without crashing the
> driver.
> 
> Remove init_completion() for "send_af_done" from brcmf_p2p_create_p2pdev()
> and do it in brcmf_p2p_tx_action_frame() instead of reinit_completion().
> Because the formar function would not get executed when hostapd is managing
> wlan interface, and so it is not safe to do reinit_completion() without any
> prior init_completion().
> 
> And in the brcmf_p2p_tx_action_frame() function, the condition check for
> P2P Presence response frame is not needed, since the wpa_supplicant is
> properly sending the P2P Presense Response frame on the P2P-GO vif instead
> of the P2P-Device vif.

Thanks, Gokul

Looks good. Respecting the vif provided by userspace is a better 
approach. So the actual issue is that hostapd does not create the 
P2P-Device interface, which is why I suggested to fallback on the 
primary vif. Overlooked the fact that we are doing a init_completion() 
in brcmf_p2p_create_p2pdev() which is needed for the action frame 
transmit. I have some comment on theat init_completion() change you made 
(see below).

Still would prefer to move the action frame transmit (afx) functionality 
out of the p2p source file, but that can be a separate rework.

Regards,
Arend

> Fixes: 18e2f61db3b7 ("brcmfmac: P2P action frame tx.")
> Signed-off-by: Gokul Sivakumar <[email protected]>
> ---
>   .../broadcom/brcm80211/brcmfmac/cfg80211.c    |  3 +-
>   .../broadcom/brcm80211/brcmfmac/p2p.c         | 28 ++++++-------------
>   .../broadcom/brcm80211/brcmfmac/p2p.h         |  3 +-
>   3 files changed, 11 insertions(+), 23 deletions(-)
[...]

> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> index 0dc9d28cd77b..c7c40dc3be08 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c

[...]

> @@ -1538,28 +1539,20 @@ int brcmf_p2p_notify_action_tx_complete(struct brcmf_if *ifp,
>    * The WLC_E_ACTION_FRAME_COMPLETE event will be received when the action
>    * frame is transmitted.
>    */
> -static s32 brcmf_p2p_tx_action_frame(struct brcmf_p2p_info *p2p,
> +static s32 brcmf_p2p_tx_action_frame(struct brcmf_if *ifp,
> +				     struct brcmf_p2p_info *p2p,
>   				     struct brcmf_fil_af_params_le *af_params)
>   {
>   	struct brcmf_pub *drvr = p2p->cfg->pub;
> -	struct brcmf_cfg80211_vif *vif;
> -	struct brcmf_p2p_action_frame *p2p_af;
>   	s32 err = 0;
>   
>   	brcmf_dbg(TRACE, "Enter\n");
>   
> -	reinit_completion(&p2p->send_af_done);
> +	init_completion(&p2p->send_af_done);

I think we should not do this here. It used to be init_completion() here 
but the kernel API changed introducing the reinit_completion() to make 
explicit distinction between init and reinit. So I would suggest to do 
the init_completion() in brcmf_p2p_attach() which is always invoked and
leave the reinit_completion() here.

>   	clear_bit(BRCMF_P2P_STATUS_ACTION_TX_COMPLETED, &p2p->status);
>   	clear_bit(BRCMF_P2P_STATUS_ACTION_TX_NOACK, &p2p->status);