mac80211: fix remain-on-channel cancel crash

commit 3fbd45ca8d1c98f3c2582ef8bc70ade42f70947b upstream.

If a ROC item is canceled just as it expires, the work
struct may be scheduled while it is running (and waiting
for the mutex). This results in it being run after being
freed, which obviously crashes.

To fix this don't free it when aborting is requested but
instead mark it as "to be freed", which makes the work a
no-op and allows freeing it outside.

Reported-by: Jouni Malinen <j@w1.fi>
Tested-by: Jouni Malinen <j@w1.fi>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 0479c64aa83cc8beb27a90dedd1340cbeb2065a2..49c48c6bae1c44bc25183a2d4db5792eb91d1da4 100644 (file)
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2499,7 +2499,7 @@ static int ieee80211_cancel_roc(struct ieee80211_local *local,
                        list_del(&dep->list);
                        mutex_unlock(&local->mtx);
 
-                       ieee80211_roc_notify_destroy(dep);
+                       ieee80211_roc_notify_destroy(dep, true);
                        return 0;
                }
 
@@ -2539,7 +2539,7 @@ static int ieee80211_cancel_roc(struct ieee80211_local *local,
                        ieee80211_start_next_roc(local);
                mutex_unlock(&local->mtx);
 
-               ieee80211_roc_notify_destroy(found);
+               ieee80211_roc_notify_destroy(found, true);
        } else {
                /* work may be pending so use it all the time */
                found->abort = true;
@@ -2549,6 +2549,8 @@ static int ieee80211_cancel_roc(struct ieee80211_local *local,
 
                /* work will clean up etc */
                flush_delayed_work(&found->work);
+               WARN_ON(!found->to_be_freed);
+               kfree(found);
        }
 
        return 0;

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 2ed065c095629403c42574872fec6a8b85819871..55d8f89bd5817a3d0d769e6c4bb6efd1f3521647 100644 (file)
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -346,6 +346,7 @@ struct ieee80211_roc_work {
        struct ieee80211_channel *chan;
 
        bool started, abort, hw_begun, notified;
+       bool to_be_freed;
 
        unsigned long hw_start_time;
 
@@ -1363,7 +1364,7 @@ void ieee80211_offchannel_return(struct ieee80211_local *local);
 void ieee80211_roc_setup(struct ieee80211_local *local);
 void ieee80211_start_next_roc(struct ieee80211_local *local);
 void ieee80211_roc_purge(struct ieee80211_sub_if_data *sdata);
-void ieee80211_roc_notify_destroy(struct ieee80211_roc_work *roc);
+void ieee80211_roc_notify_destroy(struct ieee80211_roc_work *roc, bool free);
 void ieee80211_sw_roc_work(struct work_struct *work);
 void ieee80211_handle_roc_started(struct ieee80211_roc_work *roc);
 

diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
index a3ad4c3c80a34229801bdbdd1124201485a2f671..7acbdaa77a5b614df2cd5cd4100e9a18ca421de3 100644 (file)
--- a/net/mac80211/offchannel.c
+++ b/net/mac80211/offchannel.c
@@ -299,10 +299,13 @@ void ieee80211_start_next_roc(struct ieee80211_local *local)
        }
 }
 
-void ieee80211_roc_notify_destroy(struct ieee80211_roc_work *roc)
+void ieee80211_roc_notify_destroy(struct ieee80211_roc_work *roc, bool free)
 {
        struct ieee80211_roc_work *dep, *tmp;
 
+       if (WARN_ON(roc->to_be_freed))
+               return;
+
        /* was never transmitted */
        if (roc->frame) {
                cfg80211_mgmt_tx_status(&roc->sdata->wdev,
@@ -318,9 +321,12 @@ void ieee80211_roc_notify_destroy(struct ieee80211_roc_work *roc)
                                                   GFP_KERNEL);
 
        list_for_each_entry_safe(dep, tmp, &roc->dependents, list)
-               ieee80211_roc_notify_destroy(dep);
+               ieee80211_roc_notify_destroy(dep, true);
 
-       kfree(roc);
+       if (free)
+               kfree(roc);
+       else
+               roc->to_be_freed = true;
 }
 
 void ieee80211_sw_roc_work(struct work_struct *work)
@@ -333,6 +339,9 @@ void ieee80211_sw_roc_work(struct work_struct *work)
 
        mutex_lock(&local->mtx);
 
+       if (roc->to_be_freed)
+               goto out_unlock;
+
        if (roc->abort)
                goto finish;
 
@@ -372,7 +381,7 @@ void ieee80211_sw_roc_work(struct work_struct *work)
  finish:
                list_del(&roc->list);
                started = roc->started;
-               ieee80211_roc_notify_destroy(roc);
+               ieee80211_roc_notify_destroy(roc, !roc->abort);
 
                if (started) {
                        drv_flush(local, false);
@@ -412,7 +421,7 @@ static void ieee80211_hw_roc_done(struct work_struct *work)
 
        list_del(&roc->list);
 
-       ieee80211_roc_notify_destroy(roc);
+       ieee80211_roc_notify_destroy(roc, true);
 
        /* if there's another roc, start it now */
        ieee80211_start_next_roc(local);
@@ -462,12 +471,14 @@ void ieee80211_roc_purge(struct ieee80211_sub_if_data *sdata)
        list_for_each_entry_safe(roc, tmp, &tmp_list, list) {
                if (local->ops->remain_on_channel) {
                        list_del(&roc->list);
-                       ieee80211_roc_notify_destroy(roc);
+                       ieee80211_roc_notify_destroy(roc, true);
                } else {
                        ieee80211_queue_delayed_work(&local->hw, &roc->work, 0);
 
                        /* work will clean up etc */
                        flush_delayed_work(&roc->work);
+                       WARN_ON(!roc->to_be_freed);
+                       kfree(roc);
                }
        }