* [PATCH 5.2 1/2] mwifiex: Don't abort on small, spec-compliant vendor IEs
@ 2019-06-15 0:13 Brian Norris
2019-06-15 0:13 ` [PATCH 2/2] mwifiex: use 'total_ie_len' in mwifiex_update_bss_desc_with_ie() Brian Norris
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Brian Norris @ 2019-06-15 0:13 UTC (permalink / raw)
To: Ganapathi Bhat, Nishant Sarmukadam, Amitkumar Karwar, Xinming Hu
Cc: linux-kernel, linux-wireless, Takashi Iwai, Guenter Roeck, Brian Norris
Per the 802.11 specification, vendor IEs are (at minimum) only required
to contain an OUI. A type field is also included in ieee80211.h (struct
ieee80211_vendor_ie) but doesn't appear in the specification. The
remaining fields (subtype, version) are a convention used in WMM
headers.
Thus, we should not reject vendor-specific IEs that have only the
minimum length (3 bytes) -- we should skip over them (since we only want
to match longer IEs, that match either WMM or WPA formats). We can
reject elements that don't have the minimum-required 3 byte OUI.
While we're at it, move the non-standard subtype and version fields into
the WMM structs, to avoid this confusion in the future about generic
"vendor header" attributes.
Fixes: 685c9b7750bf ("mwifiex: Abort at too short BSS descriptor element")
Cc: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
It appears that commit 685c9b7750bf is on its way to 5.2, so I labeled
this bugfix for 5.2 as well.
drivers/net/wireless/marvell/mwifiex/fw.h | 12 +++++++++---
drivers/net/wireless/marvell/mwifiex/scan.c | 18 +++++++++++-------
.../net/wireless/marvell/mwifiex/sta_ioctl.c | 4 ++--
drivers/net/wireless/marvell/mwifiex/wmm.c | 2 +-
4 files changed, 23 insertions(+), 13 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h
index b73f99dc5a72..1fb76d2f5d3f 100644
--- a/drivers/net/wireless/marvell/mwifiex/fw.h
+++ b/drivers/net/wireless/marvell/mwifiex/fw.h
@@ -1759,9 +1759,10 @@ struct mwifiex_ie_types_wmm_queue_status {
struct ieee_types_vendor_header {
u8 element_id;
u8 len;
- u8 oui[4]; /* 0~2: oui, 3: oui_type */
- u8 oui_subtype;
- u8 version;
+ struct {
+ u8 oui[3];
+ u8 oui_type;
+ } __packed oui;
} __packed;
struct ieee_types_wmm_parameter {
@@ -1775,6 +1776,9 @@ struct ieee_types_wmm_parameter {
* Version [1]
*/
struct ieee_types_vendor_header vend_hdr;
+ u8 oui_subtype;
+ u8 version;
+
u8 qos_info_bitmap;
u8 reserved;
struct ieee_types_wmm_ac_parameters ac_params[IEEE80211_NUM_ACS];
@@ -1792,6 +1796,8 @@ struct ieee_types_wmm_info {
* Version [1]
*/
struct ieee_types_vendor_header vend_hdr;
+ u8 oui_subtype;
+ u8 version;
u8 qos_info_bitmap;
} __packed;
diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index c269a0de9413..e2786ab612ca 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -1361,21 +1361,25 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
break;
case WLAN_EID_VENDOR_SPECIFIC:
- if (element_len + 2 < sizeof(vendor_ie->vend_hdr))
- return -EINVAL;
-
vendor_ie = (struct ieee_types_vendor_specific *)
current_ptr;
- if (!memcmp
- (vendor_ie->vend_hdr.oui, wpa_oui,
- sizeof(wpa_oui))) {
+ /* 802.11 requires at least 3-byte OUI. */
+ if (element_len < sizeof(vendor_ie->vend_hdr.oui.oui))
+ return -EINVAL;
+
+ /* Not long enough for a match? Skip it. */
+ if (element_len < sizeof(wpa_oui))
+ break;
+
+ if (!memcmp(&vendor_ie->vend_hdr.oui, wpa_oui,
+ sizeof(wpa_oui))) {
bss_entry->bcn_wpa_ie =
(struct ieee_types_vendor_specific *)
current_ptr;
bss_entry->wpa_offset = (u16)
(current_ptr - bss_entry->beacon_buf);
- } else if (!memcmp(vendor_ie->vend_hdr.oui, wmm_oui,
+ } else if (!memcmp(&vendor_ie->vend_hdr.oui, wmm_oui,
sizeof(wmm_oui))) {
if (total_ie_len ==
sizeof(struct ieee_types_wmm_parameter) ||
diff --git a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
index ebc0e41e5d3b..74e50566db1f 100644
--- a/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
+++ b/drivers/net/wireless/marvell/mwifiex/sta_ioctl.c
@@ -1351,7 +1351,7 @@ mwifiex_set_gen_ie_helper(struct mwifiex_private *priv, u8 *ie_data_ptr,
/* Test to see if it is a WPA IE, if not, then
* it is a gen IE
*/
- if (!memcmp(pvendor_ie->oui, wpa_oui,
+ if (!memcmp(&pvendor_ie->oui, wpa_oui,
sizeof(wpa_oui))) {
/* IE is a WPA/WPA2 IE so call set_wpa function
*/
@@ -1361,7 +1361,7 @@ mwifiex_set_gen_ie_helper(struct mwifiex_private *priv, u8 *ie_data_ptr,
goto next_ie;
}
- if (!memcmp(pvendor_ie->oui, wps_oui,
+ if (!memcmp(&pvendor_ie->oui, wps_oui,
sizeof(wps_oui))) {
/* Test to see if it is a WPS IE,
* if so, enable wps session flag
diff --git a/drivers/net/wireless/marvell/mwifiex/wmm.c b/drivers/net/wireless/marvell/mwifiex/wmm.c
index 407b9932ca4d..64916ba15df5 100644
--- a/drivers/net/wireless/marvell/mwifiex/wmm.c
+++ b/drivers/net/wireless/marvell/mwifiex/wmm.c
@@ -240,7 +240,7 @@ mwifiex_wmm_setup_queue_priorities(struct mwifiex_private *priv,
mwifiex_dbg(priv->adapter, INFO,
"info: WMM Parameter IE: version=%d,\t"
"qos_info Parameter Set Count=%d, Reserved=%#x\n",
- wmm_ie->vend_hdr.version, wmm_ie->qos_info_bitmap &
+ wmm_ie->version, wmm_ie->qos_info_bitmap &
IEEE80211_WMM_IE_AP_QOSINFO_PARAM_SET_CNT_MASK,
wmm_ie->reserved);
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] mwifiex: use 'total_ie_len' in mwifiex_update_bss_desc_with_ie()
2019-06-15 0:13 [PATCH 5.2 1/2] mwifiex: Don't abort on small, spec-compliant vendor IEs Brian Norris
@ 2019-06-15 0:13 ` Brian Norris
2019-06-17 6:14 ` Takashi Iwai
` (2 more replies)
2019-06-17 6:12 ` [PATCH 5.2 1/2] mwifiex: Don't abort on small, spec-compliant vendor IEs Takashi Iwai
2019-06-24 13:23 ` Kalle Valo
2 siblings, 3 replies; 7+ messages in thread
From: Brian Norris @ 2019-06-15 0:13 UTC (permalink / raw)
To: Ganapathi Bhat, Nishant Sarmukadam, Amitkumar Karwar, Xinming Hu
Cc: linux-kernel, linux-wireless, Takashi Iwai, Guenter Roeck, Brian Norris
This is clearer than copy/pasting the magic number '+ 2' around, and it
even saves the need for one existing comment.
Cc: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Brian Norris <briannorris@chromium.org>
---
drivers/net/wireless/marvell/mwifiex/scan.c | 14 ++++++--------
1 file changed, 6 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/marvell/mwifiex/scan.c b/drivers/net/wireless/marvell/mwifiex/scan.c
index e2786ab612ca..707e5159262f 100644
--- a/drivers/net/wireless/marvell/mwifiex/scan.c
+++ b/drivers/net/wireless/marvell/mwifiex/scan.c
@@ -1269,7 +1269,7 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
break;
case WLAN_EID_FH_PARAMS:
- if (element_len + 2 < sizeof(*fh_param_set))
+ if (total_ie_len < sizeof(*fh_param_set))
return -EINVAL;
fh_param_set =
(struct ieee_types_fh_param_set *) current_ptr;
@@ -1279,7 +1279,7 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
break;
case WLAN_EID_DS_PARAMS:
- if (element_len + 2 < sizeof(*ds_param_set))
+ if (total_ie_len < sizeof(*ds_param_set))
return -EINVAL;
ds_param_set =
(struct ieee_types_ds_param_set *) current_ptr;
@@ -1292,7 +1292,7 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
break;
case WLAN_EID_CF_PARAMS:
- if (element_len + 2 < sizeof(*cf_param_set))
+ if (total_ie_len < sizeof(*cf_param_set))
return -EINVAL;
cf_param_set =
(struct ieee_types_cf_param_set *) current_ptr;
@@ -1302,7 +1302,7 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
break;
case WLAN_EID_IBSS_PARAMS:
- if (element_len + 2 < sizeof(*ibss_param_set))
+ if (total_ie_len < sizeof(*ibss_param_set))
return -EINVAL;
ibss_param_set =
(struct ieee_types_ibss_param_set *)
@@ -1459,10 +1459,8 @@ int mwifiex_update_bss_desc_with_ie(struct mwifiex_adapter *adapter,
break;
}
- current_ptr += element_len + 2;
-
- /* Need to account for IE ID and IE Len */
- bytes_left -= (element_len + 2);
+ current_ptr += total_ie_len;
+ bytes_left -= total_ie_len;
} /* while (bytes_left > 2) */
return ret;
--
2.22.0.410.gd8fdbe21b5-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 5.2 1/2] mwifiex: Don't abort on small, spec-compliant vendor IEs
2019-06-15 0:13 [PATCH 5.2 1/2] mwifiex: Don't abort on small, spec-compliant vendor IEs Brian Norris
2019-06-15 0:13 ` [PATCH 2/2] mwifiex: use 'total_ie_len' in mwifiex_update_bss_desc_with_ie() Brian Norris
@ 2019-06-17 6:12 ` Takashi Iwai
2019-06-24 13:23 ` Kalle Valo
2 siblings, 0 replies; 7+ messages in thread
From: Takashi Iwai @ 2019-06-17 6:12 UTC (permalink / raw)
To: Brian Norris
Cc: Ganapathi Bhat, Nishant Sarmukadam, Amitkumar Karwar, Xinming Hu,
linux-kernel, linux-wireless, Takashi Iwai, Guenter Roeck
On Sat, 15 Jun 2019 02:13:20 +0200,
Brian Norris wrote:
>
> Per the 802.11 specification, vendor IEs are (at minimum) only required
> to contain an OUI. A type field is also included in ieee80211.h (struct
> ieee80211_vendor_ie) but doesn't appear in the specification. The
> remaining fields (subtype, version) are a convention used in WMM
> headers.
>
> Thus, we should not reject vendor-specific IEs that have only the
> minimum length (3 bytes) -- we should skip over them (since we only want
> to match longer IEs, that match either WMM or WPA formats). We can
> reject elements that don't have the minimum-required 3 byte OUI.
>
> While we're at it, move the non-standard subtype and version fields into
> the WMM structs, to avoid this confusion in the future about generic
> "vendor header" attributes.
>
> Fixes: 685c9b7750bf ("mwifiex: Abort at too short BSS descriptor element")
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> It appears that commit 685c9b7750bf is on its way to 5.2, so I labeled
> this bugfix for 5.2 as well.
Thanks for catching this.
Reviewed-by: Takashi Iwai <tiwai@suse.de>
Takashi
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] mwifiex: use 'total_ie_len' in mwifiex_update_bss_desc_with_ie()
2019-06-15 0:13 ` [PATCH 2/2] mwifiex: use 'total_ie_len' in mwifiex_update_bss_desc_with_ie() Brian Norris
@ 2019-06-17 6:14 ` Takashi Iwai
2019-06-25 4:51 ` Kalle Valo
2019-10-01 9:22 ` Kalle Valo
2 siblings, 0 replies; 7+ messages in thread
From: Takashi Iwai @ 2019-06-17 6:14 UTC (permalink / raw)
To: Brian Norris
Cc: Ganapathi Bhat, Nishant Sarmukadam, Amitkumar Karwar, Xinming Hu,
linux-kernel, linux-wireless, Takashi Iwai, Guenter Roeck
On Sat, 15 Jun 2019 02:13:21 +0200,
Brian Norris wrote:
>
> This is clearer than copy/pasting the magic number '+ 2' around, and it
> even saves the need for one existing comment.
>
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
Reviewed-by: Takashi Iwai <tiwai@suse.de>
thanks,
Takashi
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 5.2 1/2] mwifiex: Don't abort on small, spec-compliant vendor IEs
2019-06-15 0:13 [PATCH 5.2 1/2] mwifiex: Don't abort on small, spec-compliant vendor IEs Brian Norris
2019-06-15 0:13 ` [PATCH 2/2] mwifiex: use 'total_ie_len' in mwifiex_update_bss_desc_with_ie() Brian Norris
2019-06-17 6:12 ` [PATCH 5.2 1/2] mwifiex: Don't abort on small, spec-compliant vendor IEs Takashi Iwai
@ 2019-06-24 13:23 ` Kalle Valo
2 siblings, 0 replies; 7+ messages in thread
From: Kalle Valo @ 2019-06-24 13:23 UTC (permalink / raw)
To: Brian Norris
Cc: Ganapathi Bhat, Nishant Sarmukadam, Amitkumar Karwar, Xinming Hu,
linux-kernel, linux-wireless, Takashi Iwai, Guenter Roeck,
Brian Norris
Brian Norris <briannorris@chromium.org> wrote:
> Per the 802.11 specification, vendor IEs are (at minimum) only required
> to contain an OUI. A type field is also included in ieee80211.h (struct
> ieee80211_vendor_ie) but doesn't appear in the specification. The
> remaining fields (subtype, version) are a convention used in WMM
> headers.
>
> Thus, we should not reject vendor-specific IEs that have only the
> minimum length (3 bytes) -- we should skip over them (since we only want
> to match longer IEs, that match either WMM or WPA formats). We can
> reject elements that don't have the minimum-required 3 byte OUI.
>
> While we're at it, move the non-standard subtype and version fields into
> the WMM structs, to avoid this confusion in the future about generic
> "vendor header" attributes.
>
> Fixes: 685c9b7750bf ("mwifiex: Abort at too short BSS descriptor element")
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> Reviewed-by: Takashi Iwai <tiwai@suse.de>
Patch applied to wireless-drivers.git, thanks.
63d7ef36103d mwifiex: Don't abort on small, spec-compliant vendor IEs
--
https://patchwork.kernel.org/patch/10996895/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] mwifiex: use 'total_ie_len' in mwifiex_update_bss_desc_with_ie()
2019-06-15 0:13 ` [PATCH 2/2] mwifiex: use 'total_ie_len' in mwifiex_update_bss_desc_with_ie() Brian Norris
2019-06-17 6:14 ` Takashi Iwai
@ 2019-06-25 4:51 ` Kalle Valo
2019-10-01 9:22 ` Kalle Valo
2 siblings, 0 replies; 7+ messages in thread
From: Kalle Valo @ 2019-06-25 4:51 UTC (permalink / raw)
To: Brian Norris
Cc: Ganapathi Bhat, Nishant Sarmukadam, Amitkumar Karwar, Xinming Hu,
linux-kernel, linux-wireless, Takashi Iwai, Guenter Roeck,
Brian Norris
Brian Norris <briannorris@chromium.org> wrote:
> This is clearer than copy/pasting the magic number '+ 2' around, and it
> even saves the need for one existing comment.
>
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> Reviewed-by: Takashi Iwai <tiwai@suse.de>
This depends on:
63d7ef36103d mwifiex: Don't abort on small, spec-compliant vendor IEs
Patch set to Awaiting Upstream.
--
https://patchwork.kernel.org/patch/10996893/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] mwifiex: use 'total_ie_len' in mwifiex_update_bss_desc_with_ie()
2019-06-15 0:13 ` [PATCH 2/2] mwifiex: use 'total_ie_len' in mwifiex_update_bss_desc_with_ie() Brian Norris
2019-06-17 6:14 ` Takashi Iwai
2019-06-25 4:51 ` Kalle Valo
@ 2019-10-01 9:22 ` Kalle Valo
2 siblings, 0 replies; 7+ messages in thread
From: Kalle Valo @ 2019-10-01 9:22 UTC (permalink / raw)
To: Brian Norris
Cc: Ganapathi Bhat, Nishant Sarmukadam, Amitkumar Karwar, Xinming Hu,
linux-kernel, linux-wireless, Takashi Iwai, Guenter Roeck,
Brian Norris
Brian Norris <briannorris@chromium.org> wrote:
> This is clearer than copy/pasting the magic number '+ 2' around, and it
> even saves the need for one existing comment.
>
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> Reviewed-by: Takashi Iwai <tiwai@suse.de>
Patch applied to wireless-drivers-next.git, thanks.
0a3ce169476f mwifiex: use 'total_ie_len' in mwifiex_update_bss_desc_with_ie()
--
https://patchwork.kernel.org/patch/10996893/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-10-01 9:23 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-15 0:13 [PATCH 5.2 1/2] mwifiex: Don't abort on small, spec-compliant vendor IEs Brian Norris
2019-06-15 0:13 ` [PATCH 2/2] mwifiex: use 'total_ie_len' in mwifiex_update_bss_desc_with_ie() Brian Norris
2019-06-17 6:14 ` Takashi Iwai
2019-06-25 4:51 ` Kalle Valo
2019-10-01 9:22 ` Kalle Valo
2019-06-17 6:12 ` [PATCH 5.2 1/2] mwifiex: Don't abort on small, spec-compliant vendor IEs Takashi Iwai
2019-06-24 13:23 ` Kalle Valo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).