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@xxxxxxxxxxx>
[...]
> >
> > 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@xxxxxxxxxx>
--
Jérôme Pouiller
