[PATCH 1/1] cfg80211: Update Transition Disable policy during port authorization

[PATCH 1/1] cfg80211: Update Transition Disable policy during port authorization

[Vinayak Yadawad]

[-- Attachment #1: Type: text/plain, Size: 7547 bytes --]

In case of 4way handshake offload, transition disable policy
updated by the AP during EAPOL 3/4 is not updated to the upper layer.
This results in mismatch between transition disable policy
between the upper layer and the driver. This patch addresses this
issue by updating transition disable policy as part of port
authorization indiation.

Signed-off-by: Vinayak Yadawad <vinayak.yadawad@broadcom.com>
---
 .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c   | 2 +-
 include/net/cfg80211.h                                    | 3 ++-
 include/uapi/linux/nl80211.h                              | 3 +++
 net/wireless/core.h                                       | 4 +++-
 net/wireless/nl80211.c                                    | 7 ++++++-
 net/wireless/nl80211.h                                    | 3 ++-
 net/wireless/sme.c                                        | 8 +++++---
 net/wireless/util.c                                       | 2 +-
 8 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
index db45da33adfd..6654c4167c36 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
@@ -6028,7 +6028,7 @@ brcmf_bss_roaming_done(struct brcmf_cfg80211_info *cfg,
 	brcmf_dbg(CONN, "Report roaming result\n");
 
 	if (profile->use_fwsup == BRCMF_PROFILE_FWSUP_1X && profile->is_ft) {
-		cfg80211_port_authorized(ndev, profile->bssid, GFP_KERNEL);
+		cfg80211_port_authorized(ndev, profile->bssid, -1, GFP_KERNEL);
 		brcmf_dbg(CONN, "Report port authorized\n");
 	}
 
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 908d58393484..2fc173e88d31 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -7659,6 +7659,7 @@ void cfg80211_roamed(struct net_device *dev, struct cfg80211_roam_info *info,
  *
  * @dev: network device
  * @bssid: the BSSID of the AP
+ * @td_bitmap: transition disable policy
  * @gfp: allocation flags
  *
  * This function should be called by a driver that supports 4 way handshake
@@ -7669,7 +7670,7 @@ void cfg80211_roamed(struct net_device *dev, struct cfg80211_roam_info *info,
  * indicate the 802.11 association.
  */
 void cfg80211_port_authorized(struct net_device *dev, const u8 *bssid,
-			      gfp_t gfp);
+			      s16 td_bitmap, gfp_t gfp);
 
 /**
  * cfg80211_disconnected - notify cfg80211 that connection was dropped
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index ffb7c573e299..c81cdc6d6c86 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -2741,6 +2741,8 @@ enum nl80211_commands {
  *	When used with %NL80211_CMD_FRAME_TX_STATUS, indicates the ack RX
  *	timestamp. When used with %NL80211_CMD_FRAME RX notification, indicates
  *	the incoming frame RX timestamp.
+ * @NL80211_ATTR_TD_BITMAP: Transition Disable bitmap, for subsequent
+ *  (re)associations.
  * @NUM_NL80211_ATTR: total number of nl80211_attrs available
  * @NL80211_ATTR_MAX: highest attribute number currently defined
  * @__NL80211_ATTR_AFTER_LAST: internal use
@@ -3268,6 +3270,7 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_TX_HW_TIMESTAMP,
 	NL80211_ATTR_RX_HW_TIMESTAMP,
+	NL80211_ATTR_TD_BITMAP,
 
 	/* add attributes here, update the policy in nl80211.c */
 
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 775e16cb99ed..d36d4b775284 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -271,6 +271,7 @@ struct cfg80211_event {
 		} ij;
 		struct {
 			u8 bssid[ETH_ALEN];
+			s16 td_bitmap;
 		} pa;
 	};
 };
@@ -409,7 +410,8 @@ int cfg80211_disconnect(struct cfg80211_registered_device *rdev,
 			bool wextev);
 void __cfg80211_roamed(struct wireless_dev *wdev,
 		       struct cfg80211_roam_info *info);
-void __cfg80211_port_authorized(struct wireless_dev *wdev, const u8 *bssid);
+void __cfg80211_port_authorized(struct wireless_dev *wdev, const u8 *bssid,
+				s16 td_bitmap);
 int cfg80211_mgd_wext_connect(struct cfg80211_registered_device *rdev,
 			      struct wireless_dev *wdev);
 void cfg80211_autodisconnect_wk(struct work_struct *work);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 2705e3ee8fc4..176aa7b3bc6c 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -17841,7 +17841,8 @@ void nl80211_send_roamed(struct cfg80211_registered_device *rdev,
 }
 
 void nl80211_send_port_authorized(struct cfg80211_registered_device *rdev,
-				  struct net_device *netdev, const u8 *bssid)
+				  struct net_device *netdev, const u8 *bssid,
+				  s16 td_bitmap)
 {
 	struct sk_buff *msg;
 	void *hdr;
@@ -17861,6 +17862,10 @@ void nl80211_send_port_authorized(struct cfg80211_registered_device *rdev,
 	    nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, bssid))
 		goto nla_put_failure;
 
+	if (td_bitmap >= 0)
+		if (nla_put_s16(msg, NL80211_ATTR_TD_BITMAP, td_bitmap))
+			goto nla_put_failure;
+
 	genlmsg_end(msg, hdr);
 
 	genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy), msg, 0,
diff --git a/net/wireless/nl80211.h b/net/wireless/nl80211.h
index 855d540ddfb9..17abb0a46d8c 100644
--- a/net/wireless/nl80211.h
+++ b/net/wireless/nl80211.h
@@ -83,7 +83,8 @@ void nl80211_send_roamed(struct cfg80211_registered_device *rdev,
 			 struct net_device *netdev,
 			 struct cfg80211_roam_info *info, gfp_t gfp);
 void nl80211_send_port_authorized(struct cfg80211_registered_device *rdev,
-				  struct net_device *netdev, const u8 *bssid);
+				  struct net_device *netdev, const u8 *bssid,
+				  s16 td_bitmap);
 void nl80211_send_disconnected(struct cfg80211_registered_device *rdev,
 			       struct net_device *netdev, u16 reason,
 			       const u8 *ie, size_t ie_len, bool from_ap);
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index 27fb2a0c4052..e3150b9edb88 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -1234,7 +1234,8 @@ void cfg80211_roamed(struct net_device *dev, struct cfg80211_roam_info *info,
 }
 EXPORT_SYMBOL(cfg80211_roamed);
 
-void __cfg80211_port_authorized(struct wireless_dev *wdev, const u8 *bssid)
+void __cfg80211_port_authorized(struct wireless_dev *wdev, const u8 *bssid,
+					s16 td_bitmap)
 {
 	ASSERT_WDEV_LOCK(wdev);
 
@@ -1247,11 +1248,11 @@ void __cfg80211_port_authorized(struct wireless_dev *wdev, const u8 *bssid)
 		return;
 
 	nl80211_send_port_authorized(wiphy_to_rdev(wdev->wiphy), wdev->netdev,
-				     bssid);
+				     bssid, td_bitmap);
 }
 
 void cfg80211_port_authorized(struct net_device *dev, const u8 *bssid,
-			      gfp_t gfp)
+			      s16 td_bitmap, gfp_t gfp)
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
@@ -1267,6 +1268,7 @@ void cfg80211_port_authorized(struct net_device *dev, const u8 *bssid,
 
 	ev->type = EVENT_PORT_AUTHORIZED;
 	memcpy(ev->pa.bssid, bssid, ETH_ALEN);
+	ev->pa.td_bitmap = td_bitmap;
 
 	/*
 	 * Use the wdev event list so that if there are pending
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 2c127951764a..6a1d82c43766 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -988,7 +988,7 @@ void cfg80211_process_wdev_events(struct wireless_dev *wdev)
 			__cfg80211_leave(wiphy_to_rdev(wdev->wiphy), wdev);
 			break;
 		case EVENT_PORT_AUTHORIZED:
-			__cfg80211_port_authorized(wdev, ev->pa.bssid);
+			__cfg80211_port_authorized(wdev, ev->pa.bssid, ev->pa.td_bitmap);
 			break;
 		}
 		wdev_unlock(wdev);
-- 
2.32.0


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

Re: [PATCH 1/1] cfg80211: Update Transition Disable policy during port authorization

[Jeff Johnson]

On 8/26/2022 5:55 AM, Vinayak Yadawad wrote:

if this is a v2 it should have been annotated as such with a change log

> In case of 4way handshake offload, transition disable policy
> updated by the AP during EAPOL 3/4 is not updated to the upper layer.
> This results in mismatch between transition disable policy
> between the upper layer and the driver. This patch addresses this
> issue by updating transition disable policy as part of port
> authorization indiation.

s/indiation/indication/

> 
> Signed-off-by: Vinayak Yadawad <vinayak.yadawad@broadcom.com>
> ---
>   .../net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c   | 2 +-
>   include/net/cfg80211.h                                    | 3 ++-
>   include/uapi/linux/nl80211.h                              | 3 +++
>   net/wireless/core.h                                       | 4 +++-
>   net/wireless/nl80211.c                                    | 7 ++++++-
>   net/wireless/nl80211.h                                    | 3 ++-
>   net/wireless/sme.c                                        | 8 +++++---
>   net/wireless/util.c                                       | 2 +-
>   8 files changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> index db45da33adfd..6654c4167c36 100644
> --- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> +++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c
> @@ -6028,7 +6028,7 @@ brcmf_bss_roaming_done(struct brcmf_cfg80211_info *cfg,
>   	brcmf_dbg(CONN, "Report roaming result\n");
>   
>   	if (profile->use_fwsup == BRCMF_PROFILE_FWSUP_1X && profile->is_ft) {
> -		cfg80211_port_authorized(ndev, profile->bssid, GFP_KERNEL);
> +		cfg80211_port_authorized(ndev, profile->bssid, -1, GFP_KERNEL);
>   		brcmf_dbg(CONN, "Report port authorized\n");
>   	}
>   
> diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
> index 908d58393484..2fc173e88d31 100644
> --- a/include/net/cfg80211.h
> +++ b/include/net/cfg80211.h
> @@ -7659,6 +7659,7 @@ void cfg80211_roamed(struct net_device *dev, struct cfg80211_roam_info *info,
>    *
>    * @dev: network device
>    * @bssid: the BSSID of the AP
> + * @td_bitmap: transition disable policy
>    * @gfp: allocation flags
>    *
>    * This function should be called by a driver that supports 4 way handshake
> @@ -7669,7 +7670,7 @@ void cfg80211_roamed(struct net_device *dev, struct cfg80211_roam_info *info,
>    * indicate the 802.11 association.
>    */
>   void cfg80211_port_authorized(struct net_device *dev, const u8 *bssid,
> -			      gfp_t gfp);
> +			      s16 td_bitmap, gfp_t gfp);
>   
>   /**
>    * cfg80211_disconnected - notify cfg80211 that connection was dropped
> diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
> index ffb7c573e299..c81cdc6d6c86 100644
> --- a/include/uapi/linux/nl80211.h
> +++ b/include/uapi/linux/nl80211.h
> @@ -2741,6 +2741,8 @@ enum nl80211_commands {
>    *	When used with %NL80211_CMD_FRAME_TX_STATUS, indicates the ack RX
>    *	timestamp. When used with %NL80211_CMD_FRAME RX notification, indicates
>    *	the incoming frame RX timestamp.
> + * @NL80211_ATTR_TD_BITMAP: Transition Disable bitmap, for subsequent
> + *  (re)associations.
>    * @NUM_NL80211_ATTR: total number of nl80211_attrs available
>    * @NL80211_ATTR_MAX: highest attribute number currently defined
>    * @__NL80211_ATTR_AFTER_LAST: internal use
> @@ -3268,6 +3270,7 @@ enum nl80211_attrs {
>   
>   	NL80211_ATTR_TX_HW_TIMESTAMP,
>   	NL80211_ATTR_RX_HW_TIMESTAMP,
> +	NL80211_ATTR_TD_BITMAP,
>   
>   	/* add attributes here, update the policy in nl80211.c */

Johannes, do you want the policy updated even thought this is 
driver->userspace and hence the policy is never applied to it?

>   
> diff --git a/net/wireless/core.h b/net/wireless/core.h
> index 775e16cb99ed..d36d4b775284 100644
> --- a/net/wireless/core.h
> +++ b/net/wireless/core.h
> @@ -271,6 +271,7 @@ struct cfg80211_event {
>   		} ij;
>   		struct {
>   			u8 bssid[ETH_ALEN];
> +			s16 td_bitmap;

I know you are using -1 as an indication that the bitmap is not used, 
but using signed with a bitmap seems strange since bitops can be 
affected by sign extension. Just something that set off my Spider-Sense.

>   		} pa;
>   	};
>   };
> @@ -409,7 +410,8 @@ int cfg80211_disconnect(struct cfg80211_registered_device *rdev,
>   			bool wextev);
>   void __cfg80211_roamed(struct wireless_dev *wdev,
>   		       struct cfg80211_roam_info *info);
> -void __cfg80211_port_authorized(struct wireless_dev *wdev, const u8 *bssid);
> +void __cfg80211_port_authorized(struct wireless_dev *wdev, const u8 *bssid,
> +				s16 td_bitmap);
>   int cfg80211_mgd_wext_connect(struct cfg80211_registered_device *rdev,
>   			      struct wireless_dev *wdev);
>   void cfg80211_autodisconnect_wk(struct work_struct *work);
> diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
> index 2705e3ee8fc4..176aa7b3bc6c 100644
> --- a/net/wireless/nl80211.c
> +++ b/net/wireless/nl80211.c
> @@ -17841,7 +17841,8 @@ void nl80211_send_roamed(struct cfg80211_registered_device *rdev,
>   }
>   
>   void nl80211_send_port_authorized(struct cfg80211_registered_device *rdev,
> -				  struct net_device *netdev, const u8 *bssid)
> +				  struct net_device *netdev, const u8 *bssid,
> +				  s16 td_bitmap)
>   {
>   	struct sk_buff *msg;
>   	void *hdr;
> @@ -17861,6 +17862,10 @@ void nl80211_send_port_authorized(struct cfg80211_registered_device *rdev,
>   	    nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, bssid))
>   		goto nla_put_failure;
>   
> +	if (td_bitmap >= 0)
> +		if (nla_put_s16(msg, NL80211_ATTR_TD_BITMAP, td_bitmap))
> +			goto nla_put_failure;
> +
>   	genlmsg_end(msg, hdr);
>   
>   	genlmsg_multicast_netns(&nl80211_fam, wiphy_net(&rdev->wiphy), msg, 0,
> diff --git a/net/wireless/nl80211.h b/net/wireless/nl80211.h
> index 855d540ddfb9..17abb0a46d8c 100644
> --- a/net/wireless/nl80211.h
> +++ b/net/wireless/nl80211.h
> @@ -83,7 +83,8 @@ void nl80211_send_roamed(struct cfg80211_registered_device *rdev,
>   			 struct net_device *netdev,
>   			 struct cfg80211_roam_info *info, gfp_t gfp);
>   void nl80211_send_port_authorized(struct cfg80211_registered_device *rdev,
> -				  struct net_device *netdev, const u8 *bssid);
> +				  struct net_device *netdev, const u8 *bssid,
> +				  s16 td_bitmap);
>   void nl80211_send_disconnected(struct cfg80211_registered_device *rdev,
>   			       struct net_device *netdev, u16 reason,
>   			       const u8 *ie, size_t ie_len, bool from_ap);
> diff --git a/net/wireless/sme.c b/net/wireless/sme.c
> index 27fb2a0c4052..e3150b9edb88 100644
> --- a/net/wireless/sme.c
> +++ b/net/wireless/sme.c
> @@ -1234,7 +1234,8 @@ void cfg80211_roamed(struct net_device *dev, struct cfg80211_roam_info *info,
>   }
>   EXPORT_SYMBOL(cfg80211_roamed);
>   
> -void __cfg80211_port_authorized(struct wireless_dev *wdev, const u8 *bssid)
> +void __cfg80211_port_authorized(struct wireless_dev *wdev, const u8 *bssid,
> +					s16 td_bitmap)
>   {
>   	ASSERT_WDEV_LOCK(wdev);
>   
> @@ -1247,11 +1248,11 @@ void __cfg80211_port_authorized(struct wireless_dev *wdev, const u8 *bssid)
>   		return;
>   
>   	nl80211_send_port_authorized(wiphy_to_rdev(wdev->wiphy), wdev->netdev,
> -				     bssid);
> +				     bssid, td_bitmap);
>   }
>   
>   void cfg80211_port_authorized(struct net_device *dev, const u8 *bssid,
> -			      gfp_t gfp)
> +			      s16 td_bitmap, gfp_t gfp)
>   {
>   	struct wireless_dev *wdev = dev->ieee80211_ptr;
>   	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
> @@ -1267,6 +1268,7 @@ void cfg80211_port_authorized(struct net_device *dev, const u8 *bssid,
>   
>   	ev->type = EVENT_PORT_AUTHORIZED;
>   	memcpy(ev->pa.bssid, bssid, ETH_ALEN);
> +	ev->pa.td_bitmap = td_bitmap;
>   
>   	/*
>   	 * Use the wdev event list so that if there are pending
> diff --git a/net/wireless/util.c b/net/wireless/util.c
> index 2c127951764a..6a1d82c43766 100644
> --- a/net/wireless/util.c
> +++ b/net/wireless/util.c
> @@ -988,7 +988,7 @@ void cfg80211_process_wdev_events(struct wireless_dev *wdev)
>   			__cfg80211_leave(wiphy_to_rdev(wdev->wiphy), wdev);
>   			break;
>   		case EVENT_PORT_AUTHORIZED:
> -			__cfg80211_port_authorized(wdev, ev->pa.bssid);
> +			__cfg80211_port_authorized(wdev, ev->pa.bssid, ev->pa.td_bitmap);
>   			break;
>   		}
>   		wdev_unlock(wdev);

Re: [PATCH 1/1] cfg80211: Update Transition Disable policy during port authorization

[Johannes Berg]

On Fri, 2022-08-26 at 08:48 -0700, Jeff Johnson wrote:
> 
> > @@ -3268,6 +3270,7 @@ enum nl80211_attrs {
> >   
> >   	NL80211_ATTR_TX_HW_TIMESTAMP,
> >   	NL80211_ATTR_RX_HW_TIMESTAMP,
> > +	NL80211_ATTR_TD_BITMAP,
> >   
> >   	/* add attributes here, update the policy in nl80211.c */
> 
> Johannes, do you want the policy updated even thought this is 
> driver->userspace and hence the policy is never applied to it?

Yeah in a sense, it doesn't really matter... I think not updating is
fine, then it will likely even be rejected, at least in any new
commands.

> >   		struct {
> >   			u8 bssid[ETH_ALEN];
> > +			s16 td_bitmap;
> 
> I know you are using -1 as an indication that the bitmap is not used, 
> but using signed with a bitmap seems strange since bitops can be 
> affected by sign extension. Just something that set off my Spider-Sense.

Yeah true ... maybe a separate validity bool would've been better?
dunno.

johannes

Re: [PATCH 1/1] cfg80211: Update Transition Disable policy during port authorization

[Vinayak Yadawad]

[-- Attachment #1: Type: text/plain, Size: 1775 bytes --]

Hi Jeff, Johannes,

From your points we understand, we need a length variable and the
td_bitmap value.

As per the spec
https://www.wi-fi.org/download.php?file=/sites/default/files/private/WPA3_Specification_v3.0.pdf(
Table 4. Transition Disable KDE format),
we do have variable length of the bitmap. So we could
1. Add 2 arguments
2. One would be for the length of td_bitmap.
3. The second argument would be an u8 bitmap array depending on length
of bitmap.
Accordingly checks can be added to indicate/ignore the indication of
bitmap to upper layer.
Also the driver can update these fields as length 0 and array NULL in
case no value to be updated by the driver.

On Mon, Aug 29, 2022 at 3:22 PM Johannes Berg <johannes@sipsolutions.net> wrote:
>
> On Fri, 2022-08-26 at 08:48 -0700, Jeff Johnson wrote:
> >
> > > @@ -3268,6 +3270,7 @@ enum nl80211_attrs {
> > >
> > >     NL80211_ATTR_TX_HW_TIMESTAMP,
> > >     NL80211_ATTR_RX_HW_TIMESTAMP,
> > > +   NL80211_ATTR_TD_BITMAP,
> > >
> > >     /* add attributes here, update the policy in nl80211.c */
> >
> > Johannes, do you want the policy updated even thought this is
> > driver->userspace and hence the policy is never applied to it?
>
> Yeah in a sense, it doesn't really matter... I think not updating is
> fine, then it will likely even be rejected, at least in any new
> commands.
>
> > >             struct {
> > >                     u8 bssid[ETH_ALEN];
> > > +                   s16 td_bitmap;
> >
> > I know you are using -1 as an indication that the bitmap is not used,
> > but using signed with a bitmap seems strange since bitops can be
> > affected by sign extension. Just something that set off my Spider-Sense.
>
> Yeah true ... maybe a separate validity bool would've been better?
> dunno.
>
> johannes
>

[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]

Re: [PATCH 1/1] cfg80211: Update Transition Disable policy during port authorization

[Vinayak Yadawad]

Hi Johannes, Jeff,

I have published the version2 with the above points mentioned.
[PATCHv2,1/1] cfg80211: Update Transition Disable policy during port
authorization

On Tue, Aug 30, 2022 at 2:46 PM Vinayak Yadawad
<vinayak.yadawad@broadcom.com> wrote:
>
> Hi Jeff, Johannes,
>
> From your points we understand, we need a length variable and the
> td_bitmap value.
>
> As per the spec
> https://www.wi-fi.org/download.php?file=/sites/default/files/private/WPA3_Specification_v3.0.pdf(
> Table 4. Transition Disable KDE format),
> we do have variable length of the bitmap. So we could
> 1. Add 2 arguments
> 2. One would be for the length of td_bitmap.
> 3. The second argument would be an u8 bitmap array depending on length
> of bitmap.
> Accordingly checks can be added to indicate/ignore the indication of
> bitmap to upper layer.
> Also the driver can update these fields as length 0 and array NULL in
> case no value to be updated by the driver.
>
> On Mon, Aug 29, 2022 at 3:22 PM Johannes Berg <johannes@sipsolutions.net> wrote:
> >
> > On Fri, 2022-08-26 at 08:48 -0700, Jeff Johnson wrote:
> > >
> > > > @@ -3268,6 +3270,7 @@ enum nl80211_attrs {
> > > >
> > > >     NL80211_ATTR_TX_HW_TIMESTAMP,
> > > >     NL80211_ATTR_RX_HW_TIMESTAMP,
> > > > +   NL80211_ATTR_TD_BITMAP,
> > > >
> > > >     /* add attributes here, update the policy in nl80211.c */
> > >
> > > Johannes, do you want the policy updated even thought this is
> > > driver->userspace and hence the policy is never applied to it?
> >
> > Yeah in a sense, it doesn't really matter... I think not updating is
> > fine, then it will likely even be rejected, at least in any new
> > commands.
> >
> > > >             struct {
> > > >                     u8 bssid[ETH_ALEN];
> > > > +                   s16 td_bitmap;
> > >
> > > I know you are using -1 as an indication that the bitmap is not used,
> > > but using signed with a bitmap seems strange since bitops can be
> > > affected by sign extension. Just something that set off my Spider-Sense.
> >
> > Yeah true ... maybe a separate validity bool would've been better?
> > dunno.
> >
> > johannes
> >

Re: [PATCH 1/1] cfg80211: Update Transition Disable policy during port authorization

[Johannes Berg]

On Fri, 2022-08-26 at 18:25 +0530, Vinayak Yadawad wrote:
> 
> @@ -1267,6 +1268,7 @@ void cfg80211_port_authorized(struct net_device *dev, const u8 *bssid,
>  
>  	ev->type = EVENT_PORT_AUTHORIZED;
>  	memcpy(ev->pa.bssid, bssid, ETH_ALEN);
> +	ev->pa.td_bitmap = td_bitmap;
>  
> 

Surely this will cause some kind of use-after-free, or stack use after
stack frame return??

In the event, I guess you need to size it for the max possible bitmap
size and copy it.

(also nit somewhere: "u8 *x" instead of "u8* x")

The function argument should probably also be const.


FWIW, I didn't really mind having a fixed two-byte bitmap, but that
doesn't address the case of it being not valid.

We could just use an "int" and say "-1 for invalid, otherwise a 16-bit
bitmap value"?

johannes

Re: [PATCH 1/1] cfg80211: Update Transition Disable policy during port authorization

[Johannes Berg]

> In case of 4way handshake offload, transition disable policy
> updated by the AP during EAPOL 3/4 is not updated to the cfg80211
> layer. This results in mismatch between transition disable policy
> between the upper layer and the driver


I find this a bit confusingly worded - cfg80211 doesn't care, so it
doesn't need to be updated _in_ cfg80211. "To" cfg80211 sounds a bit
weird. Maybe just phrase that in terms of userspace?

> +++ b/include/net/cfg80211.h
> @@ -7669,7 +7669,7 @@ void cfg80211_roamed(struct net_device *dev, struct cfg80211_roam_info *info,
>   * indicate the 802.11 association.
>   */
>  void cfg80211_port_authorized(struct net_device *dev, const u8 *bssid,
> -			      gfp_t gfp);
> +			      const u8 td_bitmap, gfp_t gfp);

Missing docs.

Also missing the corresponding driver update, even if it should just
pass 0 for now in this patch to not have a lot of new driver logic here?

Or maybe it should be valid to pass -1 (i.e. make it a s16) to avoid
passing it to userspace when the driver doesn't know?

Also no point in marking the parameter const.
 
> 
> +++ b/include/uapi/linux/nl80211.h
> @@ -3268,6 +3268,7 @@ enum nl80211_attrs {
>  
>  	NL80211_ATTR_TX_HW_TIMESTAMP,
>  	NL80211_ATTR_RX_HW_TIMESTAMP,
> +	NL80211_ATTR_TD_BITMAP,

Also missing docs.

>  void nl80211_send_port_authorized(struct cfg80211_registered_device *rdev,
> -				  struct net_device *netdev, const u8 *bssid)
> +				  struct net_device *netdev, const u8 *bssid,
> +				  const u8 td_bitmap)


same here about const

>  	if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
>  	    nla_put_u32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex) ||
> -	    nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, bssid))
> +	    nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, bssid) ||
> +	    nla_put_u8(msg, NL80211_ATTR_TD_BITMAP, td_bitmap))

and yeah maybe here check for <0 or something if it should sometimes
result in a missing attribute?

johannes

[PATCH 1/1] cfg80211: Update Transition Disable policy during port authorization

[Vinayak Yadawad]

[-- Attachment #1: Type: text/plain, Size: 5868 bytes --]

In case of 4way handshake offload, transition disable policy
updated by the AP during EAPOL 3/4 is not updated to the cfg80211
layer. This results in mismatch between transition disable policy
between the upper layer and the driver. This patch addresses this
issue by updating transition disable policy as part of port
authorization indiation.

Signed-off-by: Vinayak Yadawad <vinayak.yadawad@broadcom.com>
---
 include/net/cfg80211.h       | 2 +-
 include/uapi/linux/nl80211.h | 1 +
 net/wireless/core.h          | 4 +++-
 net/wireless/nl80211.c       | 6 ++++--
 net/wireless/nl80211.h       | 3 ++-
 net/wireless/sme.c           | 8 +++++---
 net/wireless/util.c          | 2 +-
 7 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h
index 908d58393484..9c89102c85cd 100644
--- a/include/net/cfg80211.h
+++ b/include/net/cfg80211.h
@@ -7669,7 +7669,7 @@ void cfg80211_roamed(struct net_device *dev, struct cfg80211_roam_info *info,
  * indicate the 802.11 association.
  */
 void cfg80211_port_authorized(struct net_device *dev, const u8 *bssid,
-			      gfp_t gfp);
+			      const u8 td_bitmap, gfp_t gfp);
 
 /**
  * cfg80211_disconnected - notify cfg80211 that connection was dropped
diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h
index ffb7c573e299..ef0c882d214b 100644
--- a/include/uapi/linux/nl80211.h
+++ b/include/uapi/linux/nl80211.h
@@ -3268,6 +3268,7 @@ enum nl80211_attrs {
 
 	NL80211_ATTR_TX_HW_TIMESTAMP,
 	NL80211_ATTR_RX_HW_TIMESTAMP,
+	NL80211_ATTR_TD_BITMAP,
 
 	/* add attributes here, update the policy in nl80211.c */
 
diff --git a/net/wireless/core.h b/net/wireless/core.h
index 775e16cb99ed..8baa3487f67f 100644
--- a/net/wireless/core.h
+++ b/net/wireless/core.h
@@ -271,6 +271,7 @@ struct cfg80211_event {
 		} ij;
 		struct {
 			u8 bssid[ETH_ALEN];
+			u8 td_bitmap;
 		} pa;
 	};
 };
@@ -409,7 +410,8 @@ int cfg80211_disconnect(struct cfg80211_registered_device *rdev,
 			bool wextev);
 void __cfg80211_roamed(struct wireless_dev *wdev,
 		       struct cfg80211_roam_info *info);
-void __cfg80211_port_authorized(struct wireless_dev *wdev, const u8 *bssid);
+void __cfg80211_port_authorized(struct wireless_dev *wdev, const u8 *bssid,
+				const u8 td_bitmap);
 int cfg80211_mgd_wext_connect(struct cfg80211_registered_device *rdev,
 			      struct wireless_dev *wdev);
 void cfg80211_autodisconnect_wk(struct work_struct *work);
diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c
index 2705e3ee8fc4..eee3a2d6e6f7 100644
--- a/net/wireless/nl80211.c
+++ b/net/wireless/nl80211.c
@@ -17841,7 +17841,8 @@ void nl80211_send_roamed(struct cfg80211_registered_device *rdev,
 }
 
 void nl80211_send_port_authorized(struct cfg80211_registered_device *rdev,
-				  struct net_device *netdev, const u8 *bssid)
+				  struct net_device *netdev, const u8 *bssid,
+				  const u8 td_bitmap)
 {
 	struct sk_buff *msg;
 	void *hdr;
@@ -17858,7 +17859,8 @@ void nl80211_send_port_authorized(struct cfg80211_registered_device *rdev,
 
 	if (nla_put_u32(msg, NL80211_ATTR_WIPHY, rdev->wiphy_idx) ||
 	    nla_put_u32(msg, NL80211_ATTR_IFINDEX, netdev->ifindex) ||
-	    nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, bssid))
+	    nla_put(msg, NL80211_ATTR_MAC, ETH_ALEN, bssid) ||
+	    nla_put_u8(msg, NL80211_ATTR_TD_BITMAP, td_bitmap))
 		goto nla_put_failure;
 
 	genlmsg_end(msg, hdr);
diff --git a/net/wireless/nl80211.h b/net/wireless/nl80211.h
index 855d540ddfb9..5001d9d8c635 100644
--- a/net/wireless/nl80211.h
+++ b/net/wireless/nl80211.h
@@ -83,7 +83,8 @@ void nl80211_send_roamed(struct cfg80211_registered_device *rdev,
 			 struct net_device *netdev,
 			 struct cfg80211_roam_info *info, gfp_t gfp);
 void nl80211_send_port_authorized(struct cfg80211_registered_device *rdev,
-				  struct net_device *netdev, const u8 *bssid);
+				  struct net_device *netdev, const u8 *bssid,
+				  const u8 td_bitmap);
 void nl80211_send_disconnected(struct cfg80211_registered_device *rdev,
 			       struct net_device *netdev, u16 reason,
 			       const u8 *ie, size_t ie_len, bool from_ap);
diff --git a/net/wireless/sme.c b/net/wireless/sme.c
index 27fb2a0c4052..5e9db942cbbb 100644
--- a/net/wireless/sme.c
+++ b/net/wireless/sme.c
@@ -1234,7 +1234,8 @@ void cfg80211_roamed(struct net_device *dev, struct cfg80211_roam_info *info,
 }
 EXPORT_SYMBOL(cfg80211_roamed);
 
-void __cfg80211_port_authorized(struct wireless_dev *wdev, const u8 *bssid)
+void __cfg80211_port_authorized(struct wireless_dev *wdev, const u8 *bssid,
+					const u8 td_bitmap)
 {
 	ASSERT_WDEV_LOCK(wdev);
 
@@ -1247,11 +1248,11 @@ void __cfg80211_port_authorized(struct wireless_dev *wdev, const u8 *bssid)
 		return;
 
 	nl80211_send_port_authorized(wiphy_to_rdev(wdev->wiphy), wdev->netdev,
-				     bssid);
+				     bssid, td_bitmap);
 }
 
 void cfg80211_port_authorized(struct net_device *dev, const u8 *bssid,
-			      gfp_t gfp)
+			      const u8 td_bitmap, gfp_t gfp)
 {
 	struct wireless_dev *wdev = dev->ieee80211_ptr;
 	struct cfg80211_registered_device *rdev = wiphy_to_rdev(wdev->wiphy);
@@ -1267,6 +1268,7 @@ void cfg80211_port_authorized(struct net_device *dev, const u8 *bssid,
 
 	ev->type = EVENT_PORT_AUTHORIZED;
 	memcpy(ev->pa.bssid, bssid, ETH_ALEN);
+	ev->pa.td_bitmap = td_bitmap;
 
 	/*
 	 * Use the wdev event list so that if there are pending
diff --git a/net/wireless/util.c b/net/wireless/util.c
index 2c127951764a..6a1d82c43766 100644
--- a/net/wireless/util.c
+++ b/net/wireless/util.c
@@ -988,7 +988,7 @@ void cfg80211_process_wdev_events(struct wireless_dev *wdev)
 			__cfg80211_leave(wiphy_to_rdev(wdev->wiphy), wdev);
 			break;
 		case EVENT_PORT_AUTHORIZED:
-			__cfg80211_port_authorized(wdev, ev->pa.bssid);
+			__cfg80211_port_authorized(wdev, ev->pa.bssid, ev->pa.td_bitmap);
 			break;
 		}
 		wdev_unlock(wdev);
-- 
2.32.0


[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 4218 bytes --]