2008-09-08 09:08:19

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 0/3] small mac80211 updates

Hi,

This contains just patches I've previously sent, but possibly
slightly modified (I've forgotten). Can you please ignore all
patches I sent on the weekend and apply these instead? I have
already rebased my other patches to apply after the ones from
Tomas that do the disassociation fixes.

I'm resending these mostly to make it easier for you to drop
the ones I don't want applied right now.

johannes



2008-09-08 13:42:06

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 4/3] mac80211: fix action frame length checks

The action frame length checks are one too small, there's not just
an action code as the comment makes you believe, there's a category
code too, and the category code is required in each action frame
(hence part of IEEE80211_MIN_ACTION_SIZE).

Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/mesh_hwmp.c | 4 ++++
net/mac80211/mesh_plink.c | 4 ++++
net/mac80211/mlme.c | 5 +++--
3 files changed, 11 insertions(+), 2 deletions(-)

--- everything.orig/net/mac80211/mesh_hwmp.c 2008-09-08 15:37:12.000000000 +0200
+++ everything/net/mac80211/mesh_hwmp.c 2008-09-08 15:37:25.000000000 +0200
@@ -581,6 +581,10 @@ void mesh_rx_path_sel_frame(struct ieee8
size_t baselen;
u32 last_hop_metric;

+ /* need action_code */
+ if (len < IEEE80211_MIN_ACTION_SIZE + 1)
+ return;
+
baselen = (u8 *) mgmt->u.action.u.mesh_action.variable - (u8 *) mgmt;
ieee802_11_parse_elems(mgmt->u.action.u.mesh_action.variable,
len - baselen, &elems);
--- everything.orig/net/mac80211/mesh_plink.c 2008-09-08 15:37:12.000000000 +0200
+++ everything/net/mac80211/mesh_plink.c 2008-09-08 15:37:25.000000000 +0200
@@ -421,6 +421,10 @@ void mesh_rx_plink_frame(struct ieee8021
DECLARE_MAC_BUF(mac);
#endif

+ /* need action_code, aux */
+ if (len < IEEE80211_MIN_ACTION_SIZE + 3)
+ return;
+
if (is_multicast_ether_addr(mgmt->da)) {
mpl_dbg("Mesh plink: ignore frame from multicast address");
return;
--- everything.orig/net/mac80211/mlme.c 2008-09-08 15:37:17.000000000 +0200
+++ everything/net/mac80211/mlme.c 2008-09-08 15:37:25.000000000 +0200
@@ -60,7 +60,7 @@

#define ERP_INFO_USE_PROTECTION BIT(1)

-/* mgmt header + 1 byte action code */
+/* mgmt header + 1 byte category code */
#define IEEE80211_MIN_ACTION_SIZE (24 + 1)

#define IEEE80211_ADDBA_PARAM_POLICY_MASK 0x0002
@@ -2989,7 +2989,8 @@ static void ieee80211_rx_mgmt_action(str
{
struct ieee80211_local *local = sdata->local;

- if (len < IEEE80211_MIN_ACTION_SIZE)
+ /* all categories we currently handle have action_code */
+ if (len < IEEE80211_MIN_ACTION_SIZE + 1)
return;

switch (mgmt->u.action.category) {



2008-09-09 06:30:21

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH 5/3] mac80211: fix typo in action frame handling

On Mon, 2008-09-08 at 23:58 +0300, Tomas Winkler wrote:

> It doesn't really matter on which action code you doing the switch.

Oh, I know, but I think it's confusing.

> The full code looks like that
>
> switch (mgmt->u.action.u.chan_switch.action_code) {
> case WLAN_ACTION_SPCT_MSR_REQ:
> if (len < (IEEE80211_MIN_ACTION_SIZE +
> sizeof(mgmt->u.action.u.msrment_req)))
> break;
> ieee80211_sta_process_measurement_req(dev, mgmt);
> break;
> case WLAN_ACTION_SPCT_CHL_SWITCH:
> if (len < (IEEE80211_MIN_ACTION_SIZE +
> sizeof(mgmt->u.action.u.chan_switch)))
> break;
> ieee80211_sta_process_channel_switch(dev, ifsta,
> &mgmt->u.action.u.chan_switch.sw_elem);
> break;
> case WLAN_ACTION_SPCT_TPC_REQ:
> ieee80211_sta_process_tpc_req(dev, mgmt, rx_status);
> break;
> default:
> break;

Does channel switch and measurement have the same category? Can't we
better reflect that in the structure? I thought the union in the struct
had one sub-structure per category.

johannes


Attachments:
signature.asc (836.00 B)
This is a digitally signed message part

2008-09-08 20:58:40

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH 5/3] mac80211: fix typo in action frame handling

On Mon, Sep 8, 2008 at 5:31 PM, Johannes Berg <[email protected]> wrote:
> This says chan_switch.action_code but really means
> measurement.action_code, of course the actual offset in
> the frame is the same, it's just harder to understand
> this way.
>
> Signed-off-by: Johannes Berg <[email protected]>



> net/mac80211/mlme.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- everything.orig/net/mac80211/mlme.c 2008-09-08 16:29:54.000000000 +0200
> +++ everything/net/mac80211/mlme.c 2008-09-08 16:29:56.000000000 +0200
> @@ -2997,7 +2997,7 @@ static void ieee80211_rx_mgmt_action(str
> case WLAN_CATEGORY_SPECTRUM_MGMT:
> if (local->hw.conf.channel->band != IEEE80211_BAND_5GHZ)
> break;
> - switch (mgmt->u.action.u.chan_switch.action_code) {
> + switch (mgmt->u.action.u.measurement.action_code) {
> case WLAN_ACTION_SPCT_MSR_REQ:
> if (len < (IEEE80211_MIN_ACTION_SIZE +
> sizeof(mgmt->u.action.u.measurement)))
>
>
It doesn't really matter on which action code you doing the switch.
The full code looks like that

switch (mgmt->u.action.u.chan_switch.action_code) {
case WLAN_ACTION_SPCT_MSR_REQ:
if (len < (IEEE80211_MIN_ACTION_SIZE +
sizeof(mgmt->u.action.u.msrment_req)))
break;
ieee80211_sta_process_measurement_req(dev, mgmt);
break;
case WLAN_ACTION_SPCT_CHL_SWITCH:
if (len < (IEEE80211_MIN_ACTION_SIZE +
sizeof(mgmt->u.action.u.chan_switch)))
break;
ieee80211_sta_process_channel_switch(dev, ifsta,
&mgmt->u.action.u.chan_switch.sw_elem);
break;
case WLAN_ACTION_SPCT_TPC_REQ:
ieee80211_sta_process_tpc_req(dev, mgmt, rx_status);
break;
default:
break;
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2008-09-08 14:40:42

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 6/3] mac80211: move IE parsing to util file

Since IE parsing is required for the mlme and mesh code, it's
not a static function anyway, and it's much better to have it
in util rather than the overly large mlme.c

Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/mlme.c | 145 ----------------------------------------------------
net/mac80211/util.c | 144 +++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 144 insertions(+), 145 deletions(-)

--- everything.orig/net/mac80211/mlme.c 2008-09-08 16:36:43.000000000 +0200
+++ everything/net/mac80211/mlme.c 2008-09-08 16:37:58.000000000 +0200
@@ -91,151 +91,6 @@ static int ieee80211_sta_config_auth(str
static void sta_rx_agg_session_timer_expired(unsigned long data);


-void ieee802_11_parse_elems(u8 *start, size_t len,
- struct ieee802_11_elems *elems)
-{
- size_t left = len;
- u8 *pos = start;
-
- memset(elems, 0, sizeof(*elems));
- elems->ie_start = start;
- elems->total_len = len;
-
- while (left >= 2) {
- u8 id, elen;
-
- id = *pos++;
- elen = *pos++;
- left -= 2;
-
- if (elen > left)
- return;
-
- switch (id) {
- case WLAN_EID_SSID:
- elems->ssid = pos;
- elems->ssid_len = elen;
- break;
- case WLAN_EID_SUPP_RATES:
- elems->supp_rates = pos;
- elems->supp_rates_len = elen;
- break;
- case WLAN_EID_FH_PARAMS:
- elems->fh_params = pos;
- elems->fh_params_len = elen;
- break;
- case WLAN_EID_DS_PARAMS:
- elems->ds_params = pos;
- elems->ds_params_len = elen;
- break;
- case WLAN_EID_CF_PARAMS:
- elems->cf_params = pos;
- elems->cf_params_len = elen;
- break;
- case WLAN_EID_TIM:
- elems->tim = pos;
- elems->tim_len = elen;
- break;
- case WLAN_EID_IBSS_PARAMS:
- elems->ibss_params = pos;
- elems->ibss_params_len = elen;
- break;
- case WLAN_EID_CHALLENGE:
- elems->challenge = pos;
- elems->challenge_len = elen;
- break;
- case WLAN_EID_WPA:
- if (elen >= 4 && pos[0] == 0x00 && pos[1] == 0x50 &&
- pos[2] == 0xf2) {
- /* Microsoft OUI (00:50:F2) */
- if (pos[3] == 1) {
- /* OUI Type 1 - WPA IE */
- elems->wpa = pos;
- elems->wpa_len = elen;
- } else if (elen >= 5 && pos[3] == 2) {
- if (pos[4] == 0) {
- elems->wmm_info = pos;
- elems->wmm_info_len = elen;
- } else if (pos[4] == 1) {
- elems->wmm_param = pos;
- elems->wmm_param_len = elen;
- }
- }
- }
- break;
- case WLAN_EID_RSN:
- elems->rsn = pos;
- elems->rsn_len = elen;
- break;
- case WLAN_EID_ERP_INFO:
- elems->erp_info = pos;
- elems->erp_info_len = elen;
- break;
- case WLAN_EID_EXT_SUPP_RATES:
- elems->ext_supp_rates = pos;
- elems->ext_supp_rates_len = elen;
- break;
- case WLAN_EID_HT_CAPABILITY:
- elems->ht_cap_elem = pos;
- elems->ht_cap_elem_len = elen;
- break;
- case WLAN_EID_HT_EXTRA_INFO:
- elems->ht_info_elem = pos;
- elems->ht_info_elem_len = elen;
- break;
- case WLAN_EID_MESH_ID:
- elems->mesh_id = pos;
- elems->mesh_id_len = elen;
- break;
- case WLAN_EID_MESH_CONFIG:
- elems->mesh_config = pos;
- elems->mesh_config_len = elen;
- break;
- case WLAN_EID_PEER_LINK:
- elems->peer_link = pos;
- elems->peer_link_len = elen;
- break;
- case WLAN_EID_PREQ:
- elems->preq = pos;
- elems->preq_len = elen;
- break;
- case WLAN_EID_PREP:
- elems->prep = pos;
- elems->prep_len = elen;
- break;
- case WLAN_EID_PERR:
- elems->perr = pos;
- elems->perr_len = elen;
- break;
- case WLAN_EID_CHANNEL_SWITCH:
- elems->ch_switch_elem = pos;
- elems->ch_switch_elem_len = elen;
- break;
- case WLAN_EID_QUIET:
- if (!elems->quiet_elem) {
- elems->quiet_elem = pos;
- elems->quiet_elem_len = elen;
- }
- elems->num_of_quiet_elem++;
- break;
- case WLAN_EID_COUNTRY:
- elems->country_elem = pos;
- elems->country_elem_len = elen;
- break;
- case WLAN_EID_PWR_CONSTRAINT:
- elems->pwr_constr_elem = pos;
- elems->pwr_constr_elem_len = elen;
- break;
- default:
- break;
- }
-
- left -= elen;
- pos += elen;
- }
-}
-
-
static u8 * ieee80211_bss_get_ie(struct ieee80211_sta_bss *bss, u8 ie)
{
u8 *end, *pos;
--- everything.orig/net/mac80211/util.c 2008-09-08 16:36:31.000000000 +0200
+++ everything/net/mac80211/util.c 2008-09-08 16:37:58.000000000 +0200
@@ -428,3 +428,147 @@ void ieee80211_iterate_active_interfaces
rcu_read_unlock();
}
EXPORT_SYMBOL_GPL(ieee80211_iterate_active_interfaces_atomic);
+
+void ieee802_11_parse_elems(u8 *start, size_t len,
+ struct ieee802_11_elems *elems)
+{
+ size_t left = len;
+ u8 *pos = start;
+
+ memset(elems, 0, sizeof(*elems));
+ elems->ie_start = start;
+ elems->total_len = len;
+
+ while (left >= 2) {
+ u8 id, elen;
+
+ id = *pos++;
+ elen = *pos++;
+ left -= 2;
+
+ if (elen > left)
+ return;
+
+ switch (id) {
+ case WLAN_EID_SSID:
+ elems->ssid = pos;
+ elems->ssid_len = elen;
+ break;
+ case WLAN_EID_SUPP_RATES:
+ elems->supp_rates = pos;
+ elems->supp_rates_len = elen;
+ break;
+ case WLAN_EID_FH_PARAMS:
+ elems->fh_params = pos;
+ elems->fh_params_len = elen;
+ break;
+ case WLAN_EID_DS_PARAMS:
+ elems->ds_params = pos;
+ elems->ds_params_len = elen;
+ break;
+ case WLAN_EID_CF_PARAMS:
+ elems->cf_params = pos;
+ elems->cf_params_len = elen;
+ break;
+ case WLAN_EID_TIM:
+ elems->tim = pos;
+ elems->tim_len = elen;
+ break;
+ case WLAN_EID_IBSS_PARAMS:
+ elems->ibss_params = pos;
+ elems->ibss_params_len = elen;
+ break;
+ case WLAN_EID_CHALLENGE:
+ elems->challenge = pos;
+ elems->challenge_len = elen;
+ break;
+ case WLAN_EID_WPA:
+ if (elen >= 4 && pos[0] == 0x00 && pos[1] == 0x50 &&
+ pos[2] == 0xf2) {
+ /* Microsoft OUI (00:50:F2) */
+ if (pos[3] == 1) {
+ /* OUI Type 1 - WPA IE */
+ elems->wpa = pos;
+ elems->wpa_len = elen;
+ } else if (elen >= 5 && pos[3] == 2) {
+ if (pos[4] == 0) {
+ elems->wmm_info = pos;
+ elems->wmm_info_len = elen;
+ } else if (pos[4] == 1) {
+ elems->wmm_param = pos;
+ elems->wmm_param_len = elen;
+ }
+ }
+ }
+ break;
+ case WLAN_EID_RSN:
+ elems->rsn = pos;
+ elems->rsn_len = elen;
+ break;
+ case WLAN_EID_ERP_INFO:
+ elems->erp_info = pos;
+ elems->erp_info_len = elen;
+ break;
+ case WLAN_EID_EXT_SUPP_RATES:
+ elems->ext_supp_rates = pos;
+ elems->ext_supp_rates_len = elen;
+ break;
+ case WLAN_EID_HT_CAPABILITY:
+ elems->ht_cap_elem = pos;
+ elems->ht_cap_elem_len = elen;
+ break;
+ case WLAN_EID_HT_EXTRA_INFO:
+ elems->ht_info_elem = pos;
+ elems->ht_info_elem_len = elen;
+ break;
+ case WLAN_EID_MESH_ID:
+ elems->mesh_id = pos;
+ elems->mesh_id_len = elen;
+ break;
+ case WLAN_EID_MESH_CONFIG:
+ elems->mesh_config = pos;
+ elems->mesh_config_len = elen;
+ break;
+ case WLAN_EID_PEER_LINK:
+ elems->peer_link = pos;
+ elems->peer_link_len = elen;
+ break;
+ case WLAN_EID_PREQ:
+ elems->preq = pos;
+ elems->preq_len = elen;
+ break;
+ case WLAN_EID_PREP:
+ elems->prep = pos;
+ elems->prep_len = elen;
+ break;
+ case WLAN_EID_PERR:
+ elems->perr = pos;
+ elems->perr_len = elen;
+ break;
+ case WLAN_EID_CHANNEL_SWITCH:
+ elems->ch_switch_elem = pos;
+ elems->ch_switch_elem_len = elen;
+ break;
+ case WLAN_EID_QUIET:
+ if (!elems->quiet_elem) {
+ elems->quiet_elem = pos;
+ elems->quiet_elem_len = elen;
+ }
+ elems->num_of_quiet_elem++;
+ break;
+ case WLAN_EID_COUNTRY:
+ elems->country_elem = pos;
+ elems->country_elem_len = elen;
+ break;
+ case WLAN_EID_PWR_CONSTRAINT:
+ elems->pwr_constr_elem = pos;
+ elems->pwr_constr_elem_len = elen;
+ break;
+ default:
+ break;
+ }
+
+ left -= elen;
+ pos += elen;
+ }
+}



2008-09-08 14:31:53

by Johannes Berg

[permalink] [raw]
Subject: [PATCH 5/3] mac80211: fix typo in action frame handling

This says chan_switch.action_code but really means
measurement.action_code, of course the actual offset in
the frame is the same, it's just harder to understand
this way.

Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/mlme.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

--- everything.orig/net/mac80211/mlme.c 2008-09-08 16:29:54.000000000 +0200
+++ everything/net/mac80211/mlme.c 2008-09-08 16:29:56.000000000 +0200
@@ -2997,7 +2997,7 @@ static void ieee80211_rx_mgmt_action(str
case WLAN_CATEGORY_SPECTRUM_MGMT:
if (local->hw.conf.channel->band != IEEE80211_BAND_5GHZ)
break;
- switch (mgmt->u.action.u.chan_switch.action_code) {
+ switch (mgmt->u.action.u.measurement.action_code) {
case WLAN_ACTION_SPCT_MSR_REQ:
if (len < (IEEE80211_MIN_ACTION_SIZE +
sizeof(mgmt->u.action.u.measurement)))