wifi: mac80211: fix a expired vs. cancel race in roc

When the remain on channel is removed at the time it should
have expired, we have a race: the driver could be handling
the flow of the expiration while mac80211 is cancelling
that very same remain on channel request.

This wouldn't be problem in itself, but since mac80211
can send the next request to the driver in the cancellation
flow, we can get to the following situation:

           CPU0                             CPU1
expiration of roc in driver
ieee80211_remain_on_channel_expired()
                                         Cancellation of the roc
schedules a worker (hw_roc_done)
                                         Add next roc
hw_roc_done_wk runs and ends
the second roc prematurely.

Since, by design, there is only one single request sent to the
driver at a time, we can safely assume that after the cancel()
request returns from the driver, we should not handle any worker
that handles the expiration of the request.

Cancel the hw_roc_done worker after the cancellation to make
sure we start the next one with a clean slate.

Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
Signed-off-by: Gregory Greenman <gregory.greenman@intel.com>
Link: https://lore.kernel.org/r/20230928172905.4e4469be20ac.Iab0525f5cc4698acf23eab98b8b1eec02099cde0@changeid
Signed-off-by: Johannes Berg <johannes.berg@intel.com>

diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
index 071582dbe6a51f00ce242dffe8ebe631f9f87685..6c40802025733bef3f05a4cda32a6d865b951bb3 100644 (file)
--- a/net/mac80211/offchannel.c
+++ b/net/mac80211/offchannel.c
@@ -717,6 +717,23 @@ static int ieee80211_cancel_roc(struct ieee80211_local *local,
                        return ret;
                }
 
+               /*
+                * We could be racing against the notification from the driver:
+                *  + driver is handling the notification on CPU0
+                *  + user space is cancelling the remain on channel and
+                *    schedules the hw_roc_done worker.
+                *
+                *  Now hw_roc_done might start to run after the next roc will
+                *  start and mac80211 will think that this second roc has
+                *  ended prematurely.
+                *  Cancel the work to make sure that all the pending workers
+                *  have completed execution.
+                *  Note that this assumes that by the time the driver returns
+                *  from drv_cancel_remain_on_channel, it has completed all
+                *  the processing of related notifications.
+                */
+               wiphy_work_cancel(local->hw.wiphy, &local->hw_roc_done);
+
                /* TODO:
                 * if multiple items were combined here then we really shouldn't
                 * cancel them all - we should wait for as much time as needed