Hello Jérôme!

Thank you for the quick reply!

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

RSN IE missing in beacon is normal in open networks. Avoid returning -ENODEV in this case.

Steps to reproduce:

$ cat /etc/wpa_supplicant.conf network={          ssid="testNet"          mode=2          key_mgmt=NONE }

$ wpa_supplicant -iwlan0 -c /etc/wpa_supplicant.conf nl80211: Beacon set failed: -22 (Invalid argument) Failed to set beacon parameters Interface initialization failed wlan0: interface state UNINITIALIZED->DISABLED wlan0: AP-DISABLED wlan0: Unable to setup interface. Failed to initialize AP interface

After the change:

$ wpa_supplicant -iwlan0 -c /etc/wpa_supplicant.conf Successfully initialized wpa_supplicant wlan0: interface state UNINITIALIZED->ENABLED wlan0: AP-ENABLED

Good catch, thank you.

...

Cc: stable@vger.kernel.org Fixes: fe0a7776d4d1 ("wifi: wfx: fix possible NULL pointer dereference in wfx_set_mfp_ap()") Signed-off-by: Alexander Sverdlin alexander.sverdlin@siemens.com

---

drivers/net/wireless/silabs/wfx/sta.c | 5 ++++-   1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/silabs/wfx/sta.c b/drivers/net/wireless/silabs/wfx/sta.c index 216d43c8bd6e..7c04810dbf3d 100644 --- a/drivers/net/wireless/silabs/wfx/sta.c +++ b/drivers/net/wireless/silabs/wfx/sta.c @@ -352,8 +352,11 @@ static int wfx_set_mfp_ap(struct wfx_vif *wvif)

ptr = (u16 *)cfg80211_find_ie(WLAN_EID_RSN, skb->data + ieoffset,                                        skb->len - ieoffset); -       if (unlikely(!ptr)) +       if (!ptr) { +               /* No RSN IE is fine in open networks */ +               ret = 0;                  goto free_skb; +       }

ptr += pairwise_cipher_suite_count_offset;          if (WARN_ON(ptr > (u16 *)skb_tail_pointer(skb)))

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?