| Message ID | 1370241587-2609-2-git-send-email-ordex@autistici.org (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Headers | show |
On Mon, 2013-06-03 at 08:39 +0200, Antonio Quartulli wrote: > @@ -2845,6 +2852,10 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev, > rcu_read_lock(); > chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf); > > + /* if no channel was specified, use the current one */ > + if (chanctx_conf && !chan) > + chan = chanctx_conf->def.chan; > + > if (chanctx_conf) > need_offchan = chan != chanctx_conf->def.chan; > else > @@ -2852,6 +2863,12 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev, > rcu_read_unlock(); > } > > + /* at this point a channel should have been chosen */ > + if (!chan) { > + ret = -EINVAL; > + goto out_unlock; > + } > + These two changes make no sense at all. If you look at the function you'll see that "chan" isn't used at all after the check, and modifying the "check if ..." part to use the channel also doesn't make sense. 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 Mon, Jun 03, 2013 at 05:01:30PM +0200, Johannes Berg wrote: > On Mon, 2013-06-03 at 08:39 +0200, Antonio Quartulli wrote: > > > @@ -2845,6 +2852,10 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev, > > rcu_read_lock(); > > chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf); > > > > + /* if no channel was specified, use the current one */ > > + if (chanctx_conf && !chan) > > + chan = chanctx_conf->def.chan; > > + > > if (chanctx_conf) > > need_offchan = chan != chanctx_conf->def.chan; > > else > > @@ -2852,6 +2863,12 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev, > > rcu_read_unlock(); > > } > > > > + /* at this point a channel should have been chosen */ > > + if (!chan) { > > + ret = -EINVAL; > > + goto out_unlock; > > + } > > + > > These two changes make no sense at all. If you look at the function > you'll see that "chan" isn't used at all after the check, uhm? it is passed to ieee80211_start_roc_work() right after (this part has not been changed). 2904 ret = ieee80211_start_roc_work(local, sdata, chan, > and modifying > the "check if ..." part to use the channel also doesn't make sense. > to which part do you exactly refer? I'm just ensuring that the chan variable gets assigned before something accesses it. Cheers,
On Mon, 2013-06-03 at 20:03 +0200, Antonio Quartulli wrote: > On Mon, Jun 03, 2013 at 05:01:30PM +0200, Johannes Berg wrote: > > On Mon, 2013-06-03 at 08:39 +0200, Antonio Quartulli wrote: > > > > > @@ -2845,6 +2852,10 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev, > > > rcu_read_lock(); > > > chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf); > > > > > > + /* if no channel was specified, use the current one */ > > > + if (chanctx_conf && !chan) > > > + chan = chanctx_conf->def.chan; > > > + > > > if (chanctx_conf) > > > need_offchan = chan != chanctx_conf->def.chan; > > > else > > > @@ -2852,6 +2863,12 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev, > > > rcu_read_unlock(); > > > } > > > > > > + /* at this point a channel should have been chosen */ > > > + if (!chan) { > > > + ret = -EINVAL; > > > + goto out_unlock; > > > + } > > > + > > > > These two changes make no sense at all. If you look at the function > > you'll see that "chan" isn't used at all after the check, > > uhm? it is passed to ieee80211_start_roc_work() right after (this part has not > been changed). > 2904 ret = ieee80211_start_roc_work(local, sdata, chan, No, you can't get there without need_offchan. 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 Mon, Jun 03, 2013 at 08:16:03PM +0200, Johannes Berg wrote: > On Mon, 2013-06-03 at 20:03 +0200, Antonio Quartulli wrote: > > On Mon, Jun 03, 2013 at 05:01:30PM +0200, Johannes Berg wrote: > > > On Mon, 2013-06-03 at 08:39 +0200, Antonio Quartulli wrote: > > > > > > > @@ -2845,6 +2852,10 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev, > > > > rcu_read_lock(); > > > > chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf); > > > > > > > > + /* if no channel was specified, use the current one */ > > > > + if (chanctx_conf && !chan) > > > > + chan = chanctx_conf->def.chan; > > > > + > > > > if (chanctx_conf) > > > > need_offchan = chan != chanctx_conf->def.chan; > > > > else > > > > @@ -2852,6 +2863,12 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev, > > > > rcu_read_unlock(); > > > > } > > > > > > > > + /* at this point a channel should have been chosen */ > > > > + if (!chan) { > > > > + ret = -EINVAL; > > > > + goto out_unlock; > > > > + } > > > > + > > > > > > These two changes make no sense at all. If you look at the function > > > you'll see that "chan" isn't used at all after the check, > > > > uhm? it is passed to ieee80211_start_roc_work() right after (this part has not > > been changed). > > > 2904 ret = ieee80211_start_roc_work(local, sdata, chan, > > No, you can't get there without need_offchan. ah, you are right. Now I see why these two changes do not make sense at all :-) Thanks!
diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c index 9034da1..e6e41c7 100644 --- a/net/mac80211/cfg.c +++ b/net/mac80211/cfg.c @@ -2836,6 +2836,13 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev, return -EOPNOTSUPP; } + /* + * configurations requiring offchan cannot work if no channel has been + * specified + */ + if (need_offchan && !chan) + return -EINVAL; + mutex_lock(&local->mtx); /* Check if the operating channel is the requested channel */ @@ -2845,6 +2852,10 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev, rcu_read_lock(); chanctx_conf = rcu_dereference(sdata->vif.chanctx_conf); + /* if no channel was specified, use the current one */ + if (chanctx_conf && !chan) + chan = chanctx_conf->def.chan; + if (chanctx_conf) need_offchan = chan != chanctx_conf->def.chan; else @@ -2852,6 +2863,12 @@ static int ieee80211_mgmt_tx(struct wiphy *wiphy, struct wireless_dev *wdev, rcu_read_unlock(); } + /* at this point a channel should have been chosen */ + if (!chan) { + ret = -EINVAL; + goto out_unlock; + } + if (need_offchan && !offchan) { ret = -EBUSY; goto out_unlock;