On Tue, Oct 21, 2014 at 9:42 AM, Johannes Berg
<johan...@sipsolutions.net> wrote:
> On Tue, 2014-10-21 at 09:26 +0530, Sujith Manoharan wrote:
>> Johannes Berg wrote:
>> > Maybe we need something like this:
>> >
>> > diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
>> > index af0d094b2f2f..45b74ab1c59d 100644
>> > --- a/net/mac80211/scan.c
>> > +++ b/net/mac80211/scan.c
>> > @@ -985,7 +985,6 @@ void ieee80211_scan_cancel(struct ieee80211_local 
>> > *local)
>> >                     drv_cancel_hw_scan(local,
>> >                             rcu_dereference_protected(local->scan_sdata,
>> >                                             lockdep_is_held(&local->mtx)));
>> > -           goto out;
>> >     }
>>
>> Is the big comment at the beginning of ieee80211_scan_cancel()
>> outdated ? It does seem valid to me, at least the point about
>> not cancelling scan_work for HW scan.
>
> Yeah you're right.
>
> However, when the interface is deleted, the driver must still also
> remove the scan and definitely schedule the work, so most drivers will
> really always do so in cancel_hw_scan()? We could change the API and
> require that to be synchronous, like in the interface removal case, but
> maybe we don't want to.
>
> How about this then?
>
> diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
> index e469b3390f2a..6f1b90eb568c 100644
> --- a/net/mac80211/iface.c
> +++ b/net/mac80211/iface.c
> @@ -766,10 +766,12 @@ static void ieee80211_do_stop(struct 
> ieee80211_sub_if_data *sdata,
>         int i, flushed;
>         struct ps_data *ps;
>         struct cfg80211_chan_def chandef;
> +       bool cancel_scan;
>
>         clear_bit(SDATA_STATE_RUNNING, &sdata->state);
>
> -       if (rcu_access_pointer(local->scan_sdata) == sdata)
> +       cancel_scan = rcu_access_pointer(local->scan_sdata) == sdata;
> +       if (cancel_scan)
>                 ieee80211_scan_cancel(local);
>
>         /*
> @@ -993,6 +995,9 @@ static void ieee80211_do_stop(struct 
> ieee80211_sub_if_data *sdata,
>
>         ieee80211_recalc_ps(local, -1);
>
> +       if (cancel_scan)
> +               flush_delayed_work(&local->scan_work);
> +
>         if (local->open_count == 0) {
>                 ieee80211_stop_device(local);
>
This patch actually solves a kernel panic that can be reproduced
easily by increasing the delay in ieee80211_scan_completed() and
removing the driver right after initiating a scan - the delayed
scan_work never gets flushed, resulting in invalid memory access, etc.

Eliad.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to