Thanks Johannes for the review. >It seems you should validate that the length is even, and at least 2? Can I know why the check for even/ atleast 2 is? Though I would validate for a non zero length and return failure in the else, considering a case where the TDLS peer advertises a single channel. Shouldn't it suffice? > Does this even make sense in set_station() rather than only new_station()? I would have this in both new_station and set_station. Considering the current behavior of the supplicant where new_station and set_station are invoked before and after the TPK handshake, it would be fine to have only in set_station but I suppose NL changes should not rely on supplicant's behavior. Regards, Sunil -----Original Message----- From: Johannes Berg [mailto:johannes@xxxxxxxxxxxxxxxx] Sent: Wednesday, October 02, 2013 2:31 PM To: Undekari, Sunil Dutt Cc: linux-wireless@xxxxxxxxxxxxxxx; j@xxxxx Subject: Re: [PATCH] cfg80211: Pass station supported channel and oper class info to kernel On Tue, 2013-08-27 at 11:14 +0530, Sunil Dutt wrote: > + if (info->attrs[NL80211_ATTR_STA_SUPPORTED_CHANNELS]) { > + params->supported_channels = > + nla_data(info->attrs[NL80211_ATTR_STA_SUPPORTED_CHANNELS]); > + params->supported_channels_len = > + nla_len(info->attrs[NL80211_ATTR_STA_SUPPORTED_CHANNELS]); It seems you should validate that the length is even, and at least 2? > + if (info->attrs[NL80211_ATTR_STA_SUPPORTED_OPER_CLASSES]) { > + params->supported_oper_classes = > + nla_data(info->attrs[NL80211_ATTR_STA_SUPPORTED_OPER_CLASSES]); > + params->supported_oper_classes_len = > + nla_len(info->attrs[NL80211_ATTR_STA_SUPPORTED_OPER_CLASSES]); Similarly here (with different rules) Does this even make sense in set_station() rather than only new_station()? johannes ��.n��������+%������w��{.n�����{���zW����ܨ}���Ơz�j:+v�����w����ޙ��&�)ߡ�a����z�ޗ���ݢj��w�f
