On Sat, 2008-10-11 at 15:41 +0200, Michael Buesch wrote:
> On Saturday 11 October 2008 01:53:55 Johannes Berg wrote:
> > static int b43_op_config(struct ieee80211_hw *hw, u32 changed)
> > {
> > struct b43_wl *wl = hw_to_b43_wl(hw);
> > @@ -3333,6 +3349,20 @@ static int b43_op_config(struct ieee8021
> >
> > mutex_lock(&wl->mutex);
> >
> > + if (changed & IEEE80211_CONF_CHANGE_RETRY_LIMITS) {
> > + dev = wl->current_dev;
> > + if (unlikely(!dev || (b43_status(dev) < B43_STAT_INITIALIZED))) {
> > + err = -ENODEV;
> > + goto out_unlock_mutex;
> > + }
> > + b43_set_retry_limits(dev, conf->short_frame_max_tx_count,
> > + conf->long_frame_max_tx_count);
> > + changed &= ~IEEE80211_CONF_CHANGE_RETRY_LIMITS;
> > + }
> > +
> > + if (!changed)
> > + goto out_unlock_mutex;
> > +
> > /* Switch the band (if necessary). This might change the active core. */
> > err = b43_switch_band(wl, conf->channel);
> > if (err)
>
> This must be done _after_ bandswitch. (You also have a valid dev pointer there.
> Just do it after the checks done there).
Possibly, but the original code didn't do that either and I didn't want
to think much :)
> > + mutex_lock(&wl->mutex);
> > + dev = wl->current_dev;
> > +
> > + if (changed & IEEE80211_CONF_CHANGE_RETRY_LIMITS) {
> > + if (unlikely(!dev ||
> > + (b43legacy_status(dev) < B43legacy_STAT_INITIALIZED))) {
> > + err = -ENODEV;
> > + goto out_unlock_mutex;
> > + }
> > + b43legacy_set_retry_limits(dev,
> > + conf->short_frame_max_tx_count,
> > + conf->long_frame_max_tx_count);
> > + changed &= ~IEEE80211_CONF_CHANGE_RETRY_LIMITS;
> > + }
> > +
> > + if (!changed)
> > + goto out_unlock_mutex;
> > +
> > antenna_tx = B43legacy_ANTENNA_DEFAULT;
> > antenna_rx = B43legacy_ANTENNA_DEFAULT;
> >
> > - mutex_lock(&wl->mutex);
> > dev = wl->current_dev;
>
> This also is a bit fishy.
Which part? the double assignment of "wl"?
johannes
Attachment:
signature.asc
Description: This is a digitally signed message part
