Re: [PATCH] wifi: wfx: repair open network AP mode

["Jérôme Pouiller" <jerome.pouiller@silabs.com>]

On Monday 26 August 2024 17:42:28 CEST Sverdlin, Alexander wrote:
[...]
> On Mon, 2024-08-26 at 17:12 +0200, Jérôme Pouiller wrote:
> > On Friday 23 August 2024 15:15:20 CEST A. Sverdlin wrote:
> > >
> > > From: Alexander Sverdlin <alexander.sverdlin@siemens.com>

[...]

> >
> > wfx_hif_set_mfp() is no more called when open network is started. Normally,
> > wfx_hif_reset() is sufficient to avoid any side effect with previous calls
> > to wfx_hif_set_mfp().
> >
> > However, if you don't mind, I would prefer to call wfx_hif_set_mfp() in all
> > cases.
> 
> I'm a little bit confused by this comment... You write "wfx_hif_set_mfp() is no more called",
> but I struggle to find when it was last time called (for open networks).
> Not when you visited this part of the code in commit b8cfb7c819dd
> ("wifi: wfx: fix memory leak when starting AP"), not in fe0a7776d4d1
> ("wifi: wfx: fix possible NULL pointer dereference in wfx_set_mfp_ap()").
> And even not before the latter change (say, fe0a7776d4d1^):
> 
> static void wfx_set_mfp_ap(struct wfx_vif *wvif)
> {
>         struct ieee80211_vif *vif = wvif_to_vif(wvif);
>         struct sk_buff *skb = ieee80211_beacon_get(wvif->wdev->hw, vif, 0);
>         const int ieoffset = offsetof(struct ieee80211_mgmt, u.beacon.variable);
>         const u16 *ptr = (u16 *)cfg80211_find_ie(WLAN_EID_RSN, skb->data + ieoffset,
>                                                  skb->len - ieoffset);
>         const int pairwise_cipher_suite_count_offset = 8 / sizeof(u16);
>         const int pairwise_cipher_suite_size = 4 / sizeof(u16);
>         const int akm_suite_size = 4 / sizeof(u16);
> 
>         if (ptr) {
>                 ptr += pairwise_cipher_suite_count_offset;
>                 if (WARN_ON(ptr > (u16 *)skb_tail_pointer(skb)))
>                         return;
>                 ptr += 1 + pairwise_cipher_suite_size * *ptr;
>                 if (WARN_ON(ptr > (u16 *)skb_tail_pointer(skb)))
>                         return;
>                 ptr += 1 + akm_suite_size * *ptr;
>                 if (WARN_ON(ptr > (u16 *)skb_tail_pointer(skb)))
>                         return;
>                 wfx_hif_set_mfp(wvif, *ptr & BIT(7), *ptr & BIT(6));
>         }
> }
> 
> What do I miss?

Indeed, you're right. This was the original behavior. So:

Reviewed-by: Jérôme Pouiller <jerome.pouiller@silabs.com>


-- 
Jérôme Pouiller