Search Linux Wireless

Re: [PATCH v3 1/2] nl80211/cfg80211: add scan channel times to scan command

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> + * @NL80211_ATTR_SCAN_PSV_MAX_CH_TIME: Maximum passive scan time (in TUs),
> + *	u32 attribute (similar to @NL80211_ATTR_SCAN_MAX_CH_TIME).
> + *	Note:
> + *	 The above channel time attributes are for the %NL80211_CMD_TRIGGER_SCAN
> + *	 command. The attributes are optional, the driver will use default
> + *	 channel time values if the attribute is not set.
> + *	 If one of the min times will be greater than max or set to zero,
> + *	 -EINVAL will be returned. For the software scan only the min times
> + *	 are relevant.

What if the driver doesn't support the timings?

> +	if (info->attrs[NL80211_ATTR_SCAN_MIN_CH_TIME]) {
> +	if (info->attrs[NL80211_ATTR_SCAN_MAX_CH_TIME]) {
> +	if (info->attrs[NL80211_ATTR_SCAN_PSV_MIN_CH_TIME]) {
> +	if (info->attrs[NL80211_ATTR_SCAN_PSV_MAX_CH_TIME]) {

I think you should ignore the attributes if NL80211_FEATURE_SCAN_TIMES
is not set. That will enforce that anyone implementing it sets the
feature flag correctly, if they ever test their code at all anyway :-)


> +int cfg80211_trigger_scan(struct cfg80211_registered_device *rdev)
> +{
> +	struct cfg80211_scan_request *request;
> +
> +	ASSERT_RDEV_LOCK(rdev);
> +
> +	request = rdev->scan_req;
> +
> +	if (!request)
> +		return -EINVAL;
> +
> +	if (!request->min_passive_ch_time)
> +		request->min_passive_ch_time =
> +		      ieee80211_msec_to_tu(IEEE80211_PSV_CH_TIME_MSEC);
> +	if (!request->min_ch_time)
> +		request->min_ch_time =
> +			ieee80211_msec_to_tu(IEEE80211_CH_TIME_MSEC);

I pointed this out before -- this is problematic. Userspace could
request a max time of 10, and then this would set the min time to 30,
which means min > max which is completely stupid. We should prevent such
settings.

Maybe we need to enforce that if MIN is set, the respective MAX is also
set by userspace. That way, there are no surprises if userspace sets
only MAX but we change the default values at some point.


> @@ -1023,6 +1047,20 @@ int cfg80211_wext_siwscan(struct net_device *dev,
>  		if (wiphy->bands[i])
>  			creq->rates[i] = (1 << wiphy->bands[i]->n_bitrates) - 1;
>  
> +	if (wreq->min_channel_time) {
> +		creq->min_ch_time = wreq->min_channel_time;

are you sure that wext uses TUs?

johannes

--
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


[Index of Archives]     [Linux Host AP]     [ATH6KL]     [Linux Wireless Personal Area Network]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Linux Kernel]     [IDE]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite Hiking]     [MIPS Linux]     [ARM Linux]     [Linux RAID]

  Powered by Linux