| Message ID | 1360881457-30562-1-git-send-email-johannes@sipsolutions.net (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Thu, Feb 14, 2013 at 2:37 PM, Johannes Berg <johannes@sipsolutions.net> wrote: > From: Johannes Berg <johannes.berg@intel.com> > > Make the ability to leave the plink_state unchanged not use a > magic -1 variable that isn't in the enum, but an explicit change > flag; reject invalid plink states or actions and move the needed > constants for plink actions to the right header file. Also > reject plink_state changes for non-mesh interfaces. > > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c > index 909053a..8e1a03e 100644 > --- a/net/mac80211/cfg.c > +++ b/net/mac80211/cfg.c > @@ -1261,7 +1261,9 @@ static int sta_apply_parameters(struct ieee80211_local *local, > if (ieee80211_vif_is_mesh(&sdata->vif)) { > #ifdef CONFIG_MAC80211_MESH > u32 changed = 0; > - if (sdata->u.mesh.security & IEEE80211_MESH_SEC_SECURED) { > + if (sdata->u.mesh.security & IEEE80211_MESH_SEC_SECURED && > + (params->sta_modify_mask & > + STATION_PARAM_APPLY_PLINK_STATE)) { > switch (params->plink_state) { > case NL80211_PLINK_ESTAB: > if (sta->plink_state != NL80211_PLINK_ESTAB) > @@ -1292,12 +1294,18 @@ static int sta_apply_parameters(struct ieee80211_local *local, > /* nothing */ > break; > } > + } else if (params->sta_modify_mask & > + STATION_PARAM_APPLY_PLINK_STATE) { > + return -EINVAL; It would make sense to disallow setting plink_action if MPM is in userspace.
On Thu, 2013-02-14 at 14:52 -0800, Thomas Pedersen wrote: > On Thu, Feb 14, 2013 at 2:37 PM, Johannes Berg > <johannes@sipsolutions.net> wrote: > > From: Johannes Berg <johannes.berg@intel.com> > > > > Make the ability to leave the plink_state unchanged not use a > > magic -1 variable that isn't in the enum, but an explicit change > > flag; reject invalid plink states or actions and move the needed > > constants for plink actions to the right header file. Also > > reject plink_state changes for non-mesh interfaces. > > > > Signed-off-by: Johannes Berg <johannes.berg@intel.com> > > > diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c > > index 909053a..8e1a03e 100644 > > --- a/net/mac80211/cfg.c > > +++ b/net/mac80211/cfg.c > > @@ -1261,7 +1261,9 @@ static int sta_apply_parameters(struct ieee80211_local *local, > > if (ieee80211_vif_is_mesh(&sdata->vif)) { > > #ifdef CONFIG_MAC80211_MESH > > u32 changed = 0; > > - if (sdata->u.mesh.security & IEEE80211_MESH_SEC_SECURED) { > > + if (sdata->u.mesh.security & IEEE80211_MESH_SEC_SECURED && > > + (params->sta_modify_mask & > > + STATION_PARAM_APPLY_PLINK_STATE)) { > > switch (params->plink_state) { > > case NL80211_PLINK_ESTAB: > > if (sta->plink_state != NL80211_PLINK_ESTAB) > > @@ -1292,12 +1294,18 @@ static int sta_apply_parameters(struct ieee80211_local *local, > > /* nothing */ > > break; > > } > > + } else if (params->sta_modify_mask & > > + STATION_PARAM_APPLY_PLINK_STATE) { > > + return -EINVAL; > > It would make sense to disallow setting plink_action if MPM is in userspace. I'm not sure what you mean? johannes -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Feb 14, 2013 at 3:51 PM, Johannes Berg <johannes@sipsolutions.net> wrote: > On Thu, 2013-02-14 at 14:52 -0800, Thomas Pedersen wrote: >> On Thu, Feb 14, 2013 at 2:37 PM, Johannes Berg >> <johannes@sipsolutions.net> wrote: >> > From: Johannes Berg <johannes.berg@intel.com> >> > >> > Make the ability to leave the plink_state unchanged not use a >> > magic -1 variable that isn't in the enum, but an explicit change >> > flag; reject invalid plink states or actions and move the needed >> > constants for plink actions to the right header file. Also >> > reject plink_state changes for non-mesh interfaces. >> > >> > Signed-off-by: Johannes Berg <johannes.berg@intel.com> >> >> > diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c >> > index 909053a..8e1a03e 100644 >> > --- a/net/mac80211/cfg.c >> > +++ b/net/mac80211/cfg.c >> > @@ -1261,7 +1261,9 @@ static int sta_apply_parameters(struct ieee80211_local *local, >> > if (ieee80211_vif_is_mesh(&sdata->vif)) { >> > #ifdef CONFIG_MAC80211_MESH >> > u32 changed = 0; >> > - if (sdata->u.mesh.security & IEEE80211_MESH_SEC_SECURED) { >> > + if (sdata->u.mesh.security & IEEE80211_MESH_SEC_SECURED && >> > + (params->sta_modify_mask & >> > + STATION_PARAM_APPLY_PLINK_STATE)) { >> > switch (params->plink_state) { >> > case NL80211_PLINK_ESTAB: >> > if (sta->plink_state != NL80211_PLINK_ESTAB) >> > @@ -1292,12 +1294,18 @@ static int sta_apply_parameters(struct ieee80211_local *local, >> > /* nothing */ >> > break; >> > } >> > + } else if (params->sta_modify_mask & >> > + STATION_PARAM_APPLY_PLINK_STATE) { >> > + return -EINVAL; >> >> It would make sense to disallow setting plink_action if MPM is in userspace. > > I'm not sure what you mean? if mesh security is on (i.e. MPM lives in userspace), the kernel MPM won't be running and therefore the user shouldn't be able to initiate a plink_action. The way you have it now is if (mesh_is_secure && sta_modify_mask & PLINK_STATE) ... set state ... else if (sta_modify_mask & PLINK_STATE) -EINVAL } else ... do plink_action ... the case if (mesh_is_secure && sta_modify_mask & PLINK_ACTION) should return an -EINVAL as well.
diff --git a/include/net/cfg80211.h b/include/net/cfg80211.h index b518b54..ed73542 100644 --- a/include/net/cfg80211.h +++ b/include/net/cfg80211.h @@ -611,22 +611,10 @@ struct cfg80211_ap_settings { }; /** - * enum plink_action - actions to perform in mesh peers - * - * @PLINK_ACTION_INVALID: action 0 is reserved - * @PLINK_ACTION_OPEN: start mesh peer link establishment - * @PLINK_ACTION_BLOCK: block traffic from this mesh peer - */ -enum plink_actions { - PLINK_ACTION_INVALID, - PLINK_ACTION_OPEN, - PLINK_ACTION_BLOCK, -}; - -/** * enum station_parameters_apply_mask - station parameter values to apply * @STATION_PARAM_APPLY_UAPSD: apply new uAPSD parameters (uapsd_queues, max_sp) * @STATION_PARAM_APPLY_CAPABILITY: apply new capability + * @STATION_PARAM_APPLY_PLINK_STATE: apply new plink state * * Not all station parameters have in-band "no change" signalling, * for those that don't these flags will are used. @@ -634,6 +622,7 @@ enum plink_actions { enum station_parameters_apply_mask { STATION_PARAM_APPLY_UAPSD = BIT(0), STATION_PARAM_APPLY_CAPABILITY = BIT(1), + STATION_PARAM_APPLY_PLINK_STATE = BIT(2), }; /** diff --git a/include/uapi/linux/nl80211.h b/include/uapi/linux/nl80211.h index 5354cc6..5b7601f 100644 --- a/include/uapi/linux/nl80211.h +++ b/include/uapi/linux/nl80211.h @@ -890,7 +890,8 @@ enum nl80211_commands { * consisting of a nested array. * * @NL80211_ATTR_MESH_ID: mesh id (1-32 bytes). - * @NL80211_ATTR_STA_PLINK_ACTION: action to perform on the mesh peer link. + * @NL80211_ATTR_STA_PLINK_ACTION: action to perform on the mesh peer link + * (see &enum nl80211_plink_action). * @NL80211_ATTR_MPATH_NEXT_HOP: MAC address of the next hop for a mesh path. * @NL80211_ATTR_MPATH_INFO: information about a mesh_path, part of mesh path * info given for %NL80211_CMD_GET_MPATH, nested attribute described at @@ -3323,6 +3324,23 @@ enum nl80211_plink_state { MAX_NL80211_PLINK_STATES = NUM_NL80211_PLINK_STATES - 1 }; +/** + * enum nl80211_plink_action - actions to perform in mesh peers + * + * @NL80211_PLINK_ACTION_NO_ACTION: perform no action + * @NL80211_PLINK_ACTION_OPEN: start mesh peer link establishment + * @NL80211_PLINK_ACTION_BLOCK: block traffic from this mesh peer + * @NUM_NL80211_PLINK_ACTIONS: number of possible actions + */ +enum plink_actions { + NL80211_PLINK_ACTION_NO_ACTION, + NL80211_PLINK_ACTION_OPEN, + NL80211_PLINK_ACTION_BLOCK, + + NUM_NL80211_PLINK_ACTIONS, +}; + + #define NL80211_KCK_LEN 16 #define NL80211_KEK_LEN 16 #define NL80211_REPLAY_CTR_LEN 8 diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index 909053a..8e1a03e 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -1261,7 +1261,9 @@ static int sta_apply_parameters(struct ieee80211_local *local, if (ieee80211_vif_is_mesh(&sdata->vif)) { #ifdef CONFIG_MAC80211_MESH u32 changed = 0; - if (sdata->u.mesh.security & IEEE80211_MESH_SEC_SECURED) { + if (sdata->u.mesh.security & IEEE80211_MESH_SEC_SECURED && + (params->sta_modify_mask & + STATION_PARAM_APPLY_PLINK_STATE)) { switch (params->plink_state) { case NL80211_PLINK_ESTAB: if (sta->plink_state != NL80211_PLINK_ESTAB) @@ -1292,12 +1294,18 @@ static int sta_apply_parameters(struct ieee80211_local *local, /* nothing */ break; } + } else if (params->sta_modify_mask & + STATION_PARAM_APPLY_PLINK_STATE) { + return -EINVAL; } else { switch (params->plink_action) { - case PLINK_ACTION_OPEN: + case NL80211_PLINK_ACTION_NO_ACTION: + /* nothing */ + break; + case NL80211_PLINK_ACTION_OPEN: changed |= mesh_plink_open(sta); break; - case PLINK_ACTION_BLOCK: + case NL80211_PLINK_ACTION_BLOCK: changed |= mesh_plink_block(sta); break; } diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index f3a07d0..5523150 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -3632,7 +3632,6 @@ static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info) memset(¶ms, 0, sizeof(params)); params.listen_interval = -1; - params.plink_state = -1; if (info->attrs[NL80211_ATTR_STA_AID]) return -EINVAL; @@ -3671,13 +3670,20 @@ static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info) if (parse_station_flags(info, dev->ieee80211_ptr->iftype, ¶ms)) return -EINVAL; - if (info->attrs[NL80211_ATTR_STA_PLINK_ACTION]) + if (info->attrs[NL80211_ATTR_STA_PLINK_ACTION]) { params.plink_action = - nla_get_u8(info->attrs[NL80211_ATTR_STA_PLINK_ACTION]); + nla_get_u8(info->attrs[NL80211_ATTR_STA_PLINK_ACTION]); + if (params.plink_action >= NUM_NL80211_PLINK_ACTIONS) + return -EINVAL; + } - if (info->attrs[NL80211_ATTR_STA_PLINK_STATE]) + if (info->attrs[NL80211_ATTR_STA_PLINK_STATE]) { params.plink_state = - nla_get_u8(info->attrs[NL80211_ATTR_STA_PLINK_STATE]); + nla_get_u8(info->attrs[NL80211_ATTR_STA_PLINK_STATE]); + if (params.plink_state >= NUM_NL80211_PLINK_STATES) + return -EINVAL; + params.sta_modify_mask |= STATION_PARAM_APPLY_PLINK_STATE; + } if (info->attrs[NL80211_ATTR_LOCAL_MESH_POWER_MODE]) { enum nl80211_mesh_power_mode pm = nla_get_u32( @@ -3699,6 +3705,8 @@ static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info) return -EINVAL; if (params.local_pm) return -EINVAL; + if (params.sta_modify_mask & STATION_PARAM_APPLY_PLINK_STATE) + return -EINVAL; /* TDLS can't be set, ... */ if (params.sta_flags_set & BIT(NL80211_STA_FLAG_TDLS_PEER)) @@ -3762,6 +3770,8 @@ static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info) return -EINVAL; if (params.local_pm) return -EINVAL; + if (params.sta_modify_mask & STATION_PARAM_APPLY_PLINK_STATE) + return -EINVAL; /* reject any changes other than AUTHORIZED or WME (for TDLS) */ if (params.sta_flags_mask & ~(BIT(NL80211_STA_FLAG_AUTHORIZED) | BIT(NL80211_STA_FLAG_WME))) @@ -3773,6 +3783,8 @@ static int nl80211_set_station(struct sk_buff *skb, struct genl_info *info) return -EINVAL; if (params.local_pm) return -EINVAL; + if (params.sta_modify_mask & STATION_PARAM_APPLY_PLINK_STATE) + return -EINVAL; if (info->attrs[NL80211_ATTR_HT_CAPABILITY] || info->attrs[NL80211_ATTR_VHT_CAPABILITY]) return -EINVAL; @@ -3872,9 +3884,12 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info) params.vht_capa = nla_data(info->attrs[NL80211_ATTR_VHT_CAPABILITY]); - if (info->attrs[NL80211_ATTR_STA_PLINK_ACTION]) + if (info->attrs[NL80211_ATTR_STA_PLINK_ACTION]) { params.plink_action = - nla_get_u8(info->attrs[NL80211_ATTR_STA_PLINK_ACTION]); + nla_get_u8(info->attrs[NL80211_ATTR_STA_PLINK_ACTION]); + if (params.plink_action >= NUM_NL80211_PLINK_ACTIONS) + return -EINVAL; + } if (!rdev->ops->add_station) return -EOPNOTSUPP;