On Thursday 26 January 2017 03:06 PM, Johannes Berg wrote:
>
>> +static bool cfg80211_off_channel_oper_allowed(struct wireless_dev
>> *wdev)
>> +{
>> + if (!cfg80211_beaconing_iface_active(wdev))
>> + return true;
>> +
>> + if (!(wdev->chandef.chan->flags & IEEE80211_CHAN_RADAR))
>> + return true;
>
> That could use some locking assertions. Maybe also in the
> cfg80211_beaconing_iface_active() function you introduced in the
> previous patch.
Ok.
>
>> + if (!cfg80211_off_channel_oper_allowed(wdev)) {
>> + struct ieee80211_channel *chan;
>> +
>> + if (request->n_channels != 1) {
>> + err = -EBUSY;
>> + goto out_free;
>> + }
>> +
>> + chan = request->channels[0];
>> + if (chan->center_freq != wdev->chandef.chan-
>>> center_freq) {
>> + err = -EBUSY;
>> + goto out_free;
>> + }
>> + }
>
> I'm not convinced you even hold the relevant locks here, though off the
> top of my head I'm not even sure which are needed.
Thanks for pointing it, it should be within wdev_lock().
>
>> i = 0;
>> if (n_ssids) {
>> nla_for_each_nested(attr, info-
>>> attrs[NL80211_ATTR_SCAN_SSIDS], tmp) {
>> @@ -9053,6 +9079,7 @@ static int nl80211_remain_on_channel(struct
>> sk_buff *skb,
>> struct cfg80211_registered_device *rdev = info->user_ptr[0];
>> struct wireless_dev *wdev = info->user_ptr[1];
>> struct cfg80211_chan_def chandef;
>> + const struct cfg80211_chan_def *compat_chandef;
>> struct sk_buff *msg;
>> void *hdr;
>> u64 cookie;
>> @@ -9081,6 +9108,14 @@ static int nl80211_remain_on_channel(struct
>> sk_buff *skb,
>> if (err)
>> return err;
>>
>> + if (!(cfg80211_off_channel_oper_allowed(wdev) ||
>> + cfg80211_chandef_identical(&wdev->chandef, &chandef)))
>
> I'd prefer to write that as !off_channel && !chandef_identical, seems
> easier to understand here.
Ok.
Thanks,
Vasanth
