diff mbox series

WNM: Support for BSS Color Collision and BSS Color In Use of WNM event report

Message ID 20230214230758.83028-1-yi-chia.hsieh@mediatek.com
State Accepted
Headers show
Series WNM: Support for BSS Color Collision and BSS Color In Use of WNM event report | expand

Commit Message

Yi-Chia Hsieh Feb. 14, 2023, 11:07 p.m. UTC 
Add support for WNM event report to handle BSS Color Collision and In Use event.

Co-developed-by: Ryder Lee <ryder.lee@mediatek.com>
Signed-off-by: Yi-Chia Hsieh <yi-chia.hsieh@mediatek.com>
Signed-off-by: Ryder Lee <ryder.lee@mediatek.com>
---
 src/ap/wnm_ap.c              | 78 ++++++++++++++++++++++++++++++++++++
 src/common/ieee802_11_defs.h | 35 ++++++++++++++++
 2 files changed, 113 insertions(+)

Comments

Jouni Malinen Feb. 20, 2023, 9:48 p.m. UTC | #1 
On Tue, Feb 14, 2023 at 03:07:58PM -0800, Yi-Chia Hsieh wrote:
> Add support for WNM event report to handle BSS Color Collision and In Use event.

Thanks, applied with some fixes:

> diff --git a/src/ap/wnm_ap.c b/src/ap/wnm_ap.c
> +static void ieee802_11_rx_wnm_event_report(struct hostapd_data *hapd,
> +					   const u8 *addr, const u8 *buf,
> +					   size_t len)
> +{
> +	struct sta_info *sta;
> +	u8 dialog_token;
> +	struct wnm_event_report_element *report_ie;
> +
> +	if (len < 6) {
> +		wpa_printf(MSG_DEBUG, "WNM: Ignore too short WNM Event Report frame from "
> +			   MACSTR, MAC2STR(addr));
> +		return;
> +	}

I don't know where that limit of 6 is coming from, but the Event Report
element used here is quite a bit longer with the two eight octet fields
in the BSS color collision case.. In other words, this can result in
buffer read overflows.

> +	dialog_token = *buf++;

So this moves the start pointer by one without updating len..

> +	report_ie = (struct wnm_event_report_element *) buf;
> +
> +	if (report_ie->eid != WLAN_EID_EVENT_REPORT)
> +		return;
> +
> +	if (report_ie->status != WNM_STATUS_SUCCESSFUL)
> +		return;
> +
> +	wpa_printf(MSG_DEBUG, "WNM: Received WNM Event Report frame from "
> +		   MACSTR " dialog_token=%u autonomous=%s type=%d (%s)",
> +		   MAC2STR(addr), dialog_token, (!report_ie->token) ? "true" : "false",
> +		   report_ie->type, wnm_event_type2str(report_ie->type));
> +
> +	wpa_hexdump(MSG_MSGDUMP, "WNM: Event Report",
> +		    buf, len);

So that would also seem to read beyond the end of the buffer..

In addition, report_ie->len needs to be checked here as well for correct
functionality.

> +	switch (report_ie->type) {
> +	case WNM_EVENT_TYPE_BSS_COLOR_COLLISION:
> +		hostapd_switch_color(hapd->iface->bss[0],
> +				     report_ie->u.bss_color_collision.color_bitmap);

That color_bitmap was defined as u64 which is quite problematic both
from the view point of potentially unaligned 64-bit reads and byte order
issues.

This might also need to be updated to merge in reports from multiple
associated STAs and local information. This is something to consider for
additional patches in this area.

> +	case WNM_EVENT_TYPE_BSS_COLOR_INUSE:
> +		if (report_ie->u.bss_color_inuse.color > 0 &&
> +		    report_ie->u.bss_color_inuse.color < 64)
> +			hapd->color_collision_bitmap |=
> +				(1ULL << report_ie->u.bss_color_inuse.color);

This should also handle the canceling a color use report (color == 0). I
did not implement this now, but the correct behavior for this would
likely require maintaining a record of the reported color use(s) per STA
and clearing that whenever color == 0 is reported.
diff mbox series

Patch

diff --git a/src/ap/wnm_ap.c b/src/ap/wnm_ap.c
index 23a352c9b..92f7e56ae 100644
--- a/src/ap/wnm_ap.c
+++ b/src/ap/wnm_ap.c
@@ -643,6 +643,80 @@  static void ieee802_11_rx_wnm_coloc_intf_report(struct hostapd_data *hapd,
 }
 
 
+
+static const char *wnm_event_type2str(enum wnm_event_report_type wtype)
+{
+#define W2S(wtype) case WNM_EVENT_TYPE_ ## wtype: return #wtype;
+	switch (wtype) {
+	W2S(TRANSITION)
+	W2S(RSNA)
+	W2S(P2P_LINK)
+	W2S(WNM_LOG)
+	W2S(BSS_COLOR_COLLISION)
+	W2S(BSS_COLOR_INUSE)
+	}
+	return "UNKNOWN";
+#undef W2S
+}
+
+
+static void ieee802_11_rx_wnm_event_report(struct hostapd_data *hapd,
+					   const u8 *addr, const u8 *buf,
+					   size_t len)
+{
+	struct sta_info *sta;
+	u8 dialog_token;
+	struct wnm_event_report_element *report_ie;
+
+	if (len < 6) {
+		wpa_printf(MSG_DEBUG, "WNM: Ignore too short WNM Event Report frame from "
+			   MACSTR, MAC2STR(addr));
+		return;
+	}
+
+	dialog_token = *buf++;
+	report_ie = (struct wnm_event_report_element *) buf;
+
+	if (report_ie->eid != WLAN_EID_EVENT_REPORT)
+		return;
+
+	if (report_ie->status != WNM_STATUS_SUCCESSFUL)
+		return;
+
+	wpa_printf(MSG_DEBUG, "WNM: Received WNM Event Report frame from "
+		   MACSTR " dialog_token=%u autonomous=%s type=%d (%s)",
+		   MAC2STR(addr), dialog_token, (!report_ie->token) ? "true" : "false",
+		   report_ie->type, wnm_event_type2str(report_ie->type));
+
+	wpa_hexdump(MSG_MSGDUMP, "WNM: Event Report",
+		    buf, len);
+
+	sta = ap_get_sta(hapd, addr);
+	if (!sta) {
+		wpa_printf(MSG_DEBUG, "Station " MACSTR
+			   " not found for received WNM Event Report",
+			   MAC2STR(addr));
+		return;
+	}
+
+	switch (report_ie->type) {
+	case WNM_EVENT_TYPE_BSS_COLOR_COLLISION:
+		hostapd_switch_color(hapd->iface->bss[0],
+				     report_ie->u.bss_color_collision.color_bitmap);
+		return;
+	case WNM_EVENT_TYPE_BSS_COLOR_INUSE:
+		if (report_ie->u.bss_color_inuse.color > 0 &&
+		    report_ie->u.bss_color_inuse.color < 64)
+			hapd->color_collision_bitmap |=
+				(1ULL << report_ie->u.bss_color_inuse.color);
+		return;
+	default:
+		wpa_printf(MSG_DEBUG, "WNM Event Report type=%d(%s) not supported",
+			   report_ie->type, wnm_event_type2str(report_ie->type));
+	}
+}
+
+
 int ieee802_11_rx_wnm_action_ap(struct hostapd_data *hapd,
 				const struct ieee80211_mgmt *mgmt, size_t len)
 {
@@ -658,6 +732,10 @@  int ieee802_11_rx_wnm_action_ap(struct hostapd_data *hapd,
 	plen = len - IEEE80211_HDRLEN - 2;
 
 	switch (action) {
+	case WNM_EVENT_REPORT:
+		ieee802_11_rx_wnm_event_report(hapd, mgmt->sa, payload,
+					       plen);
+		return 0;
 	case WNM_BSS_TRANS_MGMT_QUERY:
 		ieee802_11_rx_bss_trans_mgmt_query(hapd, mgmt->sa, payload,
 						   plen);
diff --git a/src/common/ieee802_11_defs.h b/src/common/ieee802_11_defs.h
index 21b48ac52..edbe60393 100644
--- a/src/common/ieee802_11_defs.h
+++ b/src/common/ieee802_11_defs.h
@@ -1959,6 +1959,41 @@  enum wnm_notification_Type {
 	WNM_NOTIF_TYPE_VENDOR_SPECIFIC = 221,
 };
 
+struct wnm_event_report_element {
+	u8 eid;    /* WLAN_EID_EVENT_REPORT */
+	u8 len;
+	u8 token;
+	u8 type;
+	u8 status;
+	union {
+		struct {
+			u8 tsf[8];
+			u64 color_bitmap;
+		} STRUCT_PACKED bss_color_collision;
+		struct {
+			u8 tsf[8];
+			u8 color;
+		} STRUCT_PACKED bss_color_inuse;
+	} u;
+} STRUCT_PACKED;
+
+enum wnm_event_report_status {
+	WNM_STATUS_SUCCESSFUL = 0,
+	WNM_STATUS_REQ_FAILED = 1,
+	WNM_STATUS_REQ_REFUSED = 2,
+	WNM_STATUS_REQ_INCAPABLE = 3,
+	WNM_STATUS_FREQUENT_TRANSITION = 4,
+};
+
+enum wnm_event_report_type {
+       WNM_EVENT_TYPE_TRANSITION = 0,
+       WNM_EVENT_TYPE_RSNA = 1,
+       WNM_EVENT_TYPE_P2P_LINK = 2,
+       WNM_EVENT_TYPE_WNM_LOG = 3,
+       WNM_EVENT_TYPE_BSS_COLOR_COLLISION = 4,
+       WNM_EVENT_TYPE_BSS_COLOR_INUSE = 5,
+};
+
 /* Channel Switch modes (802.11h) */
 #define CHAN_SWITCH_MODE_ALLOW_TX	0
 #define CHAN_SWITCH_MODE_BLOCK_TX	1