[PATCH] wifi: brcmfmac: cyw: clean up PMKID and cookie code

[PATCH] wifi: brcmfmac: cyw: clean up PMKID and cookie code

[bogdan.nicolae]

From: Bogdan Nicolae <bogdan.nicolae@acm.org>

Avoid setting packet_id to cookie, which is always 0. Instead, use an
increasing atomic counter. Avoids mismatches of completion events later
in brcmf_notify_mgmt_tx_status, where packet_id != vif->mgmt_tx_id is
checked.

Also, zero out auth_status on initialization. Otherwise, garbage will
leak from the stack to the firmware (when ssid is less than 32 bytes
and/or when params->pmkid is set). Then, pass the params->pmkid to the
firmware (without it, the firmware caches a garbage PMKID on successful
authentication and denies a subsequent association request that includes
the PMKID).

Signed-off-by: Bogdan Nicolae <bogdan.nicolae@acm.org>
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c   | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
index ce09d44fa..99f8c5bef 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
@@ -23,6 +23,8 @@
 #define MGMT_AUTH_FRAME_DWELL_TIME	4000
 #define MGMT_AUTH_FRAME_WAIT_TIME	(MGMT_AUTH_FRAME_DWELL_TIME + 100)
 
+static atomic_t brcmf_cyw_mgmt_tx_id = ATOMIC_INIT(0);
+
 static int brcmf_cyw_set_sae_pwd(struct brcmf_if *ifp,
 				 struct cfg80211_crypto_settings *crypto)
 {
@@ -123,7 +125,7 @@ int brcmf_cyw_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 	if (!ieee80211_is_auth(mgmt->frame_control))
 		return brcmf_cfg80211_mgmt_tx(wiphy, wdev, params, cookie);
 
-	*cookie = 0;
+	*cookie = atomic_inc_return(&brcmf_cyw_mgmt_tx_id);
 	vif = container_of(wdev, struct brcmf_cfg80211_vif, wdev);
 
 	reinit_completion(&vif->mgmt_tx);
@@ -200,7 +202,7 @@ brcmf_cyw_external_auth(struct wiphy *wiphy, struct net_device *dev,
 {
 	struct brcmf_if *ifp;
 	struct brcmf_pub *drvr;
-	struct brcmf_auth_req_status_le auth_status;
+	struct brcmf_auth_req_status_le auth_status = {};
 	int ret = 0;
 
 	brcmf_dbg(TRACE, "Enter\n");
@@ -208,6 +210,8 @@ brcmf_cyw_external_auth(struct wiphy *wiphy, struct net_device *dev,
 	ifp = netdev_priv(dev);
 	drvr = ifp->drvr;
 	if (params->status == WLAN_STATUS_SUCCESS) {
+		if (params->pmkid)
+			memcpy(auth_status.pmkid, params->pmkid, WLAN_PMKID_LEN);
 		auth_status.flags = cpu_to_le16(BRCMF_EXTAUTH_SUCCESS);
 	} else {
 		bphy_err(drvr, "External authentication failed: status=%d\n",
-- 
2.55.0

Re: [PATCH] wifi: brcmfmac: cyw: clean up PMKID and cookie code

[Arend van Spriel]

On Tue, 14 Jul 2026, bogdan.nicolae@gmail.com wrote:

> [PATCH] wifi: brcmfmac: cyw: clean up PMKID and cookie code

The code itself is correct, but there are process issues that need to be
fixed before I can accept this:

1. This is a resubmission of the patch sent on 2026-07-09 [1]. A
   resubmission must carry a version tag: the subject line should read
   "[PATCH v2] wifi: brcmfmac: ..." not "[PATCH] wifi: brcmfmac: ...".

2. A v2 submission must include a "Changes in v2:" section below the ---
   line documenting what changed from v1.

3. The From: address (bogdan.nicolae@gmail.com) does not match the
   Signed-off-by address (bogdan.nicolae@acm.org). Please use a
   consistent address or add a second Signed-off-by for the sending
   address.

4. The patch addresses two independent issues: fixing the packet_id in
   brcmf_cyw_mgmt_tx() (SAE auth frame TX) and fixing the zero-init and
   PMKID copy in brcmf_cyw_external_auth() (auth completion). Please
   split these into two separate patches.

Please send a v2 addressing these points.

[1] https://lore.kernel.org/linux-wireless/20260709122315.11400-1-bogdan.nicolae@gmail.com/

Regards,
Arend

Re: [PATCH] wifi: brcmfmac: cyw: clean up PMKID and cookie code

[Bogdan Nicolae]

Arend,

Sounds good. Do you prefer two patches in a single email or two separate emails?

Best,
Bogdan

On Mon, Jul 20, 2026 at 2:49 PM Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
>
> On Tue, 14 Jul 2026, bogdan.nicolae@gmail.com wrote:
>
> > [PATCH] wifi: brcmfmac: cyw: clean up PMKID and cookie code
>
> The code itself is correct, but there are process issues that need to be
> fixed before I can accept this:
>
> 1. This is a resubmission of the patch sent on 2026-07-09 [1]. A
>    resubmission must carry a version tag: the subject line should read
>    "[PATCH v2] wifi: brcmfmac: ..." not "[PATCH] wifi: brcmfmac: ...".
>
> 2. A v2 submission must include a "Changes in v2:" section below the ---
>    line documenting what changed from v1.
>
> 3. The From: address (bogdan.nicolae@gmail.com) does not match the
>    Signed-off-by address (bogdan.nicolae@acm.org). Please use a
>    consistent address or add a second Signed-off-by for the sending
>    address.
>
> 4. The patch addresses two independent issues: fixing the packet_id in
>    brcmf_cyw_mgmt_tx() (SAE auth frame TX) and fixing the zero-init and
>    PMKID copy in brcmf_cyw_external_auth() (auth completion). Please
>    split these into two separate patches.
>
> Please send a v2 addressing these points.
>
> [1] https://lore.kernel.org/linux-wireless/20260709122315.11400-1-bogdan.nicolae@gmail.com/
>
> Regards,
> Arend

Re: [PATCH] wifi: brcmfmac: cyw: clean up PMKID and cookie code

[Arend van Spriel]

On 20/07/2026 23:30, Bogdan Nicolae wrote:
> Arend,
> 
> Sounds good. Do you prefer two patches in a single email or two separate emails?

Way easier to deal with separate emails as we use patchwork to track them.

Regards,
Arend

[PATCH] wifi: brcmfmac: cyw: clean up PMKID and cookie code

[Bogdan Nicolae]

Avoid setting packet_id to cookie, which is always 0. Instead, use an
increasing atomic counter. Avoids mismatches of completion events later
in brcmf_notify_mgmt_tx_status, where packet_id != vif->mgmt_tx_id is
checked.

Also, zero out auth_status on initialization. Otherwise, garbage will
leak from the stack to the firmware (when bssid is less than 32 bytes
and/or when params->pmkid is set). Then, pass the params->pmkid to the
firmware (without it, the firmware caches a garbage PMKID on successful
authentication and denies a subsequent association request that includes
the PMKID).

Signed-off-by: Bogdan Nicolae <bogdan.nicolae@acm.org>
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c   | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
index ce09d44fa..cca53ff19 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
@@ -23,6 +23,8 @@
 #define MGMT_AUTH_FRAME_DWELL_TIME	4000
 #define MGMT_AUTH_FRAME_WAIT_TIME	(MGMT_AUTH_FRAME_DWELL_TIME + 100)
 
+static atomic_t brcmf_cyw_mgmt_tx_id = ATOMIC_INIT(0);
+
 static int brcmf_cyw_set_sae_pwd(struct brcmf_if *ifp,
 				 struct cfg80211_crypto_settings *crypto)
 {
@@ -155,7 +157,7 @@ int brcmf_cyw_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 
 	memcpy(&mf_params->da[0], &mgmt->da[0], ETH_ALEN);
 	memcpy(&mf_params->bssid[0], &mgmt->bssid[0], ETH_ALEN);
-	mf_params->packet_id = cpu_to_le32(*cookie);
+	mf_params->packet_id = cpu_to_le32(atomic_inc_return(&brcmf_cyw_mgmt_tx_id));
 	memcpy(mf_params->data, &buf[DOT11_MGMT_HDR_LEN],
 	       le16_to_cpu(mf_params->len));
 
@@ -200,7 +202,7 @@ brcmf_cyw_external_auth(struct wiphy *wiphy, struct net_device *dev,
 {
 	struct brcmf_if *ifp;
 	struct brcmf_pub *drvr;
-	struct brcmf_auth_req_status_le auth_status;
+	struct brcmf_auth_req_status_le auth_status = {};
 	int ret = 0;
 
 	brcmf_dbg(TRACE, "Enter\n");
@@ -208,6 +210,8 @@ brcmf_cyw_external_auth(struct wiphy *wiphy, struct net_device *dev,
 	ifp = netdev_priv(dev);
 	drvr = ifp->drvr;
 	if (params->status == WLAN_STATUS_SUCCESS) {
+		if (params->pmkid)
+			memcpy(auth_status.pmkid, params->pmkid, WLAN_PMKID_LEN);
 		auth_status.flags = cpu_to_le16(BRCMF_EXTAUTH_SUCCESS);
 	} else {
 		bphy_err(drvr, "External authentication failed: status=%d\n",
-- 
2.55.0

Re: [PATCH] wifi: brcmfmac: cyw: clean up PMKID and cookie code

[Arend van Spriel]

On 09/07/2026 14:23, Bogdan Nicolae wrote:
> Avoid setting packet_id to cookie, which is always 0. Instead, use an
> increasing atomic counter. Avoids mismatches of completion events later
> in brcmf_notify_mgmt_tx_status, where packet_id != vif->mgmt_tx_id is
> checked.
> 
> Also, zero out auth_status on initialization. Otherwise, garbage will
> leak from the stack to the firmware (when bssid is less than 32 bytes
> and/or when params->pmkid is set). Then, pass the params->pmkid to the
> firmware (without it, the firmware caches a garbage PMKID on successful
> authentication and denies a subsequent association request that includes
> the PMKID).
> 
> Signed-off-by: Bogdan Nicolae <bogdan.nicolae@acm.org>

I always get a bit confused when people use different email addresses to 
send and sign-off patches.
   > ---
>   .../net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c   | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
> index ce09d44fa..cca53ff19 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
> @@ -23,6 +23,8 @@
>   #define MGMT_AUTH_FRAME_DWELL_TIME	4000
>   #define MGMT_AUTH_FRAME_WAIT_TIME	(MGMT_AUTH_FRAME_DWELL_TIME + 100)
>   
> +static atomic_t brcmf_cyw_mgmt_tx_id = ATOMIC_INIT(0);
> +
>   static int brcmf_cyw_set_sae_pwd(struct brcmf_if *ifp,
>   				 struct cfg80211_crypto_settings *crypto)
>   {
> @@ -155,7 +157,7 @@ int brcmf_cyw_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
>   
>   	memcpy(&mf_params->da[0], &mgmt->da[0], ETH_ALEN);
>   	memcpy(&mf_params->bssid[0], &mgmt->bssid[0], ETH_ALEN);
> -	mf_params->packet_id = cpu_to_le32(*cookie);
> +	mf_params->packet_id = cpu_to_le32(atomic_inc_return(&brcmf_cyw_mgmt_tx_id));

As I understand things the cookie value here is an output parameter. The 
driver should assign it:

+	*cookie = atomic_inc_return(&brcmf_cyw_mgmt_tx_id);
	mf_params->packet_id = cpu_to_le32(*cookie);

>   	memcpy(mf_params->data, &buf[DOT11_MGMT_HDR_LEN],
>   	       le16_to_cpu(mf_params->len));
>
Regards,
Arend

Re: [PATCH] wifi: brcmfmac: cyw: clean up PMKID and cookie code

[Bogdan Nicolae]

Hi Arend,

The code already sets *cookie=0 in the beginning, which according to
my understanding tells the driver it does not require async tracking
upstream. This is correct, because the function calls
wait_for_completion_timeout, so mgmt tx completes by the time we exit
the function and doesn't need to be tracked upstream. However, while
we are waiting for mgmt tx to finish, the firmware may generate
non-matching completion events captured by notify_mgmt_tx_status.
That's why setting a non-zero packed_id (sent to and returned by
firmware as part of a completion event) and checking against
vif->mgmt_tx_id is important.

Cheers,
Bogdan

On Sun, Jul 12, 2026 at 5:30 PM Arend van Spriel
<arend.vanspriel@broadcom.com> wrote:
>
> On 09/07/2026 14:23, Bogdan Nicolae wrote:
> > Avoid setting packet_id to cookie, which is always 0. Instead, use an
> > increasing atomic counter. Avoids mismatches of completion events later
> > in brcmf_notify_mgmt_tx_status, where packet_id != vif->mgmt_tx_id is
> > checked.
> >
> > Also, zero out auth_status on initialization. Otherwise, garbage will
> > leak from the stack to the firmware (when bssid is less than 32 bytes
> > and/or when params->pmkid is set). Then, pass the params->pmkid to the
> > firmware (without it, the firmware caches a garbage PMKID on successful
> > authentication and denies a subsequent association request that includes
> > the PMKID).
> >
> > Signed-off-by: Bogdan Nicolae <bogdan.nicolae@acm.org>
>
> I always get a bit confused when people use different email addresses to
> send and sign-off patches.
>    > ---
> >   .../net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c   | 8 ++++++--
> >   1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
> > index ce09d44fa..cca53ff19 100644
> > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
> > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
> > @@ -23,6 +23,8 @@
> >   #define MGMT_AUTH_FRAME_DWELL_TIME  4000
> >   #define MGMT_AUTH_FRAME_WAIT_TIME   (MGMT_AUTH_FRAME_DWELL_TIME + 100)
> >
> > +static atomic_t brcmf_cyw_mgmt_tx_id = ATOMIC_INIT(0);
> > +
> >   static int brcmf_cyw_set_sae_pwd(struct brcmf_if *ifp,
> >                                struct cfg80211_crypto_settings *crypto)
> >   {
> > @@ -155,7 +157,7 @@ int brcmf_cyw_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
> >
> >       memcpy(&mf_params->da[0], &mgmt->da[0], ETH_ALEN);
> >       memcpy(&mf_params->bssid[0], &mgmt->bssid[0], ETH_ALEN);
> > -     mf_params->packet_id = cpu_to_le32(*cookie);
> > +     mf_params->packet_id = cpu_to_le32(atomic_inc_return(&brcmf_cyw_mgmt_tx_id));
>
> As I understand things the cookie value here is an output parameter. The
> driver should assign it:
>
> +       *cookie = atomic_inc_return(&brcmf_cyw_mgmt_tx_id);
>         mf_params->packet_id = cpu_to_le32(*cookie);
>
> >       memcpy(mf_params->data, &buf[DOT11_MGMT_HDR_LEN],
> >              le16_to_cpu(mf_params->len));
> >
> Regards,
> Arend

Re: [PATCH] wifi: brcmfmac: cyw: clean up PMKID and cookie code

[Gokul Sivakumar]

On 07/14, Bogdan Nicolae wrote:
> Hi Arend,
> 
> The code already sets *cookie=0 in the beginning, which according to
> my understanding tells the driver it does not require async tracking
> upstream. This is correct, because the function calls
> wait_for_completion_timeout, so mgmt tx completes by the time we exit
> the function and doesn't need to be tracked upstream. However, while
> we are waiting for mgmt tx to finish, the firmware may generate
> non-matching completion events captured by notify_mgmt_tx_status.
> That's why setting a non-zero packed_id (sent to and returned by
> firmware as part of a completion event) and checking against
> vif->mgmt_tx_id is important.

Avoid top-posting. Need to use interleaved style.
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

> Cheers,
> Bogdan
> 
> On Sun, Jul 12, 2026 at 5:30 PM Arend van Spriel
> <arend.vanspriel@broadcom.com> wrote:
> >
> > On 09/07/2026 14:23, Bogdan Nicolae wrote:
> > > Avoid setting packet_id to cookie, which is always 0. Instead, use an
> > > increasing atomic counter. Avoids mismatches of completion events later
> > > in brcmf_notify_mgmt_tx_status, where packet_id != vif->mgmt_tx_id is
> > > checked.
> > >
> > > Also, zero out auth_status on initialization. Otherwise, garbage will
> > > leak from the stack to the firmware (when bssid is less than 32 bytes

BSSID will always be less than 32, you must have meant to say "ssid" here
instead of "bssid". Also the auth_req_status struct does not have any "bssid"
member, it only has an "ssid" struct member. Need to fix in commit message.

> > > and/or when params->pmkid is set). Then, pass the params->pmkid to the
> > > firmware (without it, the firmware caches a garbage PMKID on successful
> > > authentication and denies a subsequent association request that includes
> > > the PMKID).
> > >
> > > Signed-off-by: Bogdan Nicolae <bogdan.nicolae@acm.org>
> >
> > I always get a bit confused when people use different email addresses to
> > send and sign-off patches.
> >    > ---
> > >   .../net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c   | 8 ++++++--
> > >   1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
> > > index ce09d44fa..cca53ff19 100644
> > > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
> > > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
> > > @@ -23,6 +23,8 @@
> > >   #define MGMT_AUTH_FRAME_DWELL_TIME  4000
> > >   #define MGMT_AUTH_FRAME_WAIT_TIME   (MGMT_AUTH_FRAME_DWELL_TIME + 100)
> > >
> > > +static atomic_t brcmf_cyw_mgmt_tx_id = ATOMIC_INIT(0);
> > > +
> > >   static int brcmf_cyw_set_sae_pwd(struct brcmf_if *ifp,
> > >                                struct cfg80211_crypto_settings *crypto)
> > >   {
> > > @@ -155,7 +157,7 @@ int brcmf_cyw_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
> > >
> > >       memcpy(&mf_params->da[0], &mgmt->da[0], ETH_ALEN);
> > >       memcpy(&mf_params->bssid[0], &mgmt->bssid[0], ETH_ALEN);
> > > -     mf_params->packet_id = cpu_to_le32(*cookie);
> > > +     mf_params->packet_id = cpu_to_le32(atomic_inc_return(&brcmf_cyw_mgmt_tx_id));
> >
> > As I understand things the cookie value here is an output parameter. The
> > driver should assign it:
> >
> > +       *cookie = atomic_inc_return(&brcmf_cyw_mgmt_tx_id);
> >         mf_params->packet_id = cpu_to_le32(*cookie);

Agree with Arend's suggestion here. It is not correct to leave *cookie = 0 as it
is and only assign a different packet_id while sending requst to firmware, because
in that case the driver will not return the real cookie for this TX to cfg80211.
Driver should assign the generated ID to *cookie and then copy that into packet_id.


Gokul

> > >       memcpy(mf_params->data, &buf[DOT11_MGMT_HDR_LEN],
> > >              le16_to_cpu(mf_params->len));
> > >
> > Regards,
> > Arend
> 

Re: [PATCH] wifi: brcmfmac: cyw: clean up PMKID and cookie code

[Bogdan Nicolae]

On Tue, Jul 14, 2026 at 8:03 AM Gokul Sivakumar
<gokulkumar.sivakumar@infineon.com> wrote:
>
> On 07/14, Bogdan Nicolae wrote:
> > Hi Arend,
> >
> > The code already sets *cookie=0 in the beginning, which according to
> > my understanding tells the driver it does not require async tracking
> > upstream. This is correct, because the function calls
> > wait_for_completion_timeout, so mgmt tx completes by the time we exit
> > the function and doesn't need to be tracked upstream. However, while
> > we are waiting for mgmt tx to finish, the firmware may generate
> > non-matching completion events captured by notify_mgmt_tx_status.
> > That's why setting a non-zero packed_id (sent to and returned by
> > firmware as part of a completion event) and checking against
> > vif->mgmt_tx_id is important.
>
> Avoid top-posting. Need to use interleaved style.
> https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
>
> > Cheers,
> > Bogdan
> >
> > On Sun, Jul 12, 2026 at 5:30 PM Arend van Spriel
> > <arend.vanspriel@broadcom.com> wrote:
> > >
> > > On 09/07/2026 14:23, Bogdan Nicolae wrote:
> > > > Avoid setting packet_id to cookie, which is always 0. Instead, use an
> > > > increasing atomic counter. Avoids mismatches of completion events later
> > > > in brcmf_notify_mgmt_tx_status, where packet_id != vif->mgmt_tx_id is
> > > > checked.
> > > >
> > > > Also, zero out auth_status on initialization. Otherwise, garbage will
> > > > leak from the stack to the firmware (when bssid is less than 32 bytes
>
> BSSID will always be less than 32, you must have meant to say "ssid" here
> instead of "bssid". Also the auth_req_status struct does not have any "bssid"
> member, it only has an "ssid" struct member. Need to fix in commit message.

Fixed

> > > > and/or when params->pmkid is set). Then, pass the params->pmkid to the
> > > > firmware (without it, the firmware caches a garbage PMKID on successful
> > > > authentication and denies a subsequent association request that includes
> > > > the PMKID).
> > > >
> > > > Signed-off-by: Bogdan Nicolae <bogdan.nicolae@acm.org>
> > >
> > > I always get a bit confused when people use different email addresses to
> > > send and sign-off patches.
> > >    > ---
> > > >   .../net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c   | 8 ++++++--
> > > >   1 file changed, 6 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
> > > > index ce09d44fa..cca53ff19 100644
> > > > --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
> > > > +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
> > > > @@ -23,6 +23,8 @@
> > > >   #define MGMT_AUTH_FRAME_DWELL_TIME  4000
> > > >   #define MGMT_AUTH_FRAME_WAIT_TIME   (MGMT_AUTH_FRAME_DWELL_TIME + 100)
> > > >
> > > > +static atomic_t brcmf_cyw_mgmt_tx_id = ATOMIC_INIT(0);
> > > > +
> > > >   static int brcmf_cyw_set_sae_pwd(struct brcmf_if *ifp,
> > > >                                struct cfg80211_crypto_settings *crypto)
> > > >   {
> > > > @@ -155,7 +157,7 @@ int brcmf_cyw_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
> > > >
> > > >       memcpy(&mf_params->da[0], &mgmt->da[0], ETH_ALEN);
> > > >       memcpy(&mf_params->bssid[0], &mgmt->bssid[0], ETH_ALEN);
> > > > -     mf_params->packet_id = cpu_to_le32(*cookie);
> > > > +     mf_params->packet_id = cpu_to_le32(atomic_inc_return(&brcmf_cyw_mgmt_tx_id));
> > >
> > > As I understand things the cookie value here is an output parameter. The
> > > driver should assign it:
> > >
> > > +       *cookie = atomic_inc_return(&brcmf_cyw_mgmt_tx_id);
> > >         mf_params->packet_id = cpu_to_le32(*cookie);
>
> Agree with Arend's suggestion here. It is not correct to leave *cookie = 0 as it
> is and only assign a different packet_id while sending requst to firmware, because
> in that case the driver will not return the real cookie for this TX to cfg80211.
> Driver should assign the generated ID to *cookie and then copy that into packet_id.

Fixed

>
>
> Gokul
>
> > > >       memcpy(mf_params->data, &buf[DOT11_MGMT_HDR_LEN],
> > > >              le16_to_cpu(mf_params->len));
> > > >
> > > Regards,
> > > Arend

Regards,
Bogdan

[PATCH] wifi: brcmfmac: cyw: clean up PMKID and cookie code

[Bogdan Nicolae]

Avoid setting packet_id to cookie, which is always 0. Instead, use an
increasing atomic counter. Avoids mismatches of completion events later
in brcmf_notify_mgmt_tx_status, where packet_id != vif->mgmt_tx_id is
checked.

Also, zero out auth_status on initialization. Otherwise, garbage will
leak from the stack to the firmware (when bssid is less than 32 bytes
and/or when params->pmkid is set). Then, pass the params->pmkid to the
firmware (without it, the firmware caches a garbage PMKID on successful
authentication and denies a subsequent association request that includes
the PMKID).

Signed-off-by: Bogdan Nicolae <bogdan.nicolae@acm.org>
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c   | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
index ce09d44fa..cca53ff19 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cyw/core.c
@@ -23,6 +23,8 @@
 #define MGMT_AUTH_FRAME_DWELL_TIME	4000
 #define MGMT_AUTH_FRAME_WAIT_TIME	(MGMT_AUTH_FRAME_DWELL_TIME + 100)
 
+static atomic_t brcmf_cyw_mgmt_tx_id = ATOMIC_INIT(0);
+
 static int brcmf_cyw_set_sae_pwd(struct brcmf_if *ifp,
 				 struct cfg80211_crypto_settings *crypto)
 {
@@ -155,7 +157,7 @@ int brcmf_cyw_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev,
 
 	memcpy(&mf_params->da[0], &mgmt->da[0], ETH_ALEN);
 	memcpy(&mf_params->bssid[0], &mgmt->bssid[0], ETH_ALEN);
-	mf_params->packet_id = cpu_to_le32(*cookie);
+	mf_params->packet_id = cpu_to_le32(atomic_inc_return(&brcmf_cyw_mgmt_tx_id));
 	memcpy(mf_params->data, &buf[DOT11_MGMT_HDR_LEN],
 	       le16_to_cpu(mf_params->len));
 
@@ -200,7 +202,7 @@ brcmf_cyw_external_auth(struct wiphy *wiphy, struct net_device *dev,
 {
 	struct brcmf_if *ifp;
 	struct brcmf_pub *drvr;
-	struct brcmf_auth_req_status_le auth_status;
+	struct brcmf_auth_req_status_le auth_status = {};
 	int ret = 0;
 
 	brcmf_dbg(TRACE, "Enter\n");
@@ -208,6 +210,8 @@ brcmf_cyw_external_auth(struct wiphy *wiphy, struct net_device *dev,
 	ifp = netdev_priv(dev);
 	drvr = ifp->drvr;
 	if (params->status == WLAN_STATUS_SUCCESS) {
+		if (params->pmkid)
+			memcpy(auth_status.pmkid, params->pmkid, WLAN_PMKID_LEN);
 		auth_status.flags = cpu_to_le16(BRCMF_EXTAUTH_SUCCESS);
 	} else {
 		bphy_err(drvr, "External authentication failed: status=%d\n",
-- 
2.55.0