On Fri, May 24, 2013 at 10:39:03PM +0200, Johannes Berg wrote: > On Thu, 2013-05-16 at 20:11 +0300, Jouni Malinen wrote: > > > - if (!info->attrs[NL80211_ATTR_STA_AID]) > > + if (!info->attrs[NL80211_ATTR_STA_AID] && > > + !info->attrs[NL80211_ATTR_PEER_AID]) > > return -EINVAL; > > Technically here I think you could check that this attribute isn't used > with non-TDLS stations, since in new_station you know what it's going to > be, right? Well, in this function yes, but not this early.. I'll add validation for this within the iftype switch. > > @@ -3985,7 +3989,10 @@ static int nl80211_new_station(struct sk_buff *skb, struct genl_info *info) > > - params.aid = nla_get_u16(info->attrs[NL80211_ATTR_STA_AID]); > > + if (info->attrs[NL80211_ATTR_STA_AID]) > > + params.aid = nla_get_u16(info->attrs[NL80211_ATTR_STA_AID]); > > + else > > + params.aid = nla_get_u16(info->attrs[NL80211_ATTR_PEER_AID]); > > This is a bit strange? Wouldn't you want to prefer the new attribute, so > you can put some possibly invalid value into the old attribute for old > kernels? Otherwise what's the reason for even reading the new attribute > in this code at all, if new userspace code must set the old attribute > for old kernels anyway? This was something to allow the new attribute to be taken into use if we ever get rid of the extra dummy STA for TDLS. I'd assume this ends up having to get a new capability flag, so wpa_supplicant could figure out which attribute to use. Anyway, I agree it makes more sense to reverse the order here to prefer _PEER_AID. -- Jouni Malinen PGP id EFC895FA -- To unsubscribe from this list: send the line "unsubscribe linux-wireless" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
