mac80211: fix PS-poll response, race

When a station queries us for a PS-poll response, we wrongly
queue the frame on the virtual interface's queue rather than
the pending queue.

Additionally, fix a race condition where we could potentially
send multiple frames to the sleeping station due to using a
station flag rather than a packet flag. When converting to a
packet flag, we can also convert p54 and remove the filter
clearing we added for it.

(Also remove a now dead function)

Signed-off-by: Johannes Berg <johannes@sipsolutions.net>
Reported-by: Bob Copeland <me@bobcopeland.com>
Tested-by: Bob Copeland <me@bobcopeland.com>
Cc: Christian Lamparter <chunkeey@web.de>
Signed-off-by: John W. Linville <linville@tuxdriver.com>

5 files changed, 12 insertions, 37 deletions

diff --git a/drivers/net/wireless/p54/txrx.c b/drivers/net/wireless/p54/txrx.c
index 0d589d68e547..c32a0d2fa1f7 100644
--- a/drivers/net/wireless/p54/txrx.c
+++ b/drivers/net/wireless/p54/txrx.c
@@ -614,7 +614,7 @@ static void p54_tx_80211_header(struct p54_common *priv, struct sk_buff *skb,
         if (info->flags & IEEE80211_TX_CTL_ASSIGN_SEQ)
                 *flags |= P54_HDR_FLAG_DATA_OUT_SEQNR;
 
-        if (info->flags & IEEE80211_TX_CTL_CLEAR_PS_FILT)
+        if (info->flags & IEEE80211_TX_CTL_PSPOLL_RESPONSE)
                 *flags |= P54_HDR_FLAG_DATA_OUT_NOCANCEL;
 
         *queue = skb_get_queue_mapping(skb) + P54_QUEUE_DATA;
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 7dd67a1ff4d5..d4e09a06b4a2 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -243,6 +243,9 @@ struct ieee80211_bss_conf {
  *      used to indicate that a frame was already retried due to PS
  * @IEEE80211_TX_INTFL_DONT_ENCRYPT: completely internal to mac80211,
  *      used to indicate frame should not be encrypted
+ * @IEEE80211_TX_CTL_PSPOLL_RESPONSE: (internal?)
+ *      This frame is a response to a PS-poll frame and should be sent
+ *      although the station is in powersave mode.
  */
 enum mac80211_tx_control_flags {
         IEEE80211_TX_CTL_REQ_TX_STATUS          = BIT(0),
@@ -262,6 +265,7 @@ enum mac80211_tx_control_flags {
         IEEE80211_TX_INTFL_NEED_TXPROCESSING    = BIT(14),
         IEEE80211_TX_INTFL_RETRIED              = BIT(15),
         IEEE80211_TX_INTFL_DONT_ENCRYPT         = BIT(16),
+        IEEE80211_TX_CTL_PSPOLL_RESPONSE        = BIT(17),
 };
 
 /**
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index cb95a3116034..f195705146bd 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -783,7 +783,7 @@ static void ap_sta_ps_start(struct sta_info *sta)
         struct ieee80211_local *local = sdata->local;
 
         atomic_inc(&sdata->bss->num_sta_ps);
-        set_and_clear_sta_flags(sta, WLAN_STA_PS, WLAN_STA_PSPOLL);
+        set_sta_flags(sta, WLAN_STA_PS);
         drv_sta_notify(local, &sdata->vif, STA_NOTIFY_SLEEP, &sta->sta);
 #ifdef CONFIG_MAC80211_VERBOSE_PS_DEBUG
         printk(KERN_DEBUG "%s: STA %pM aid %d enters power save mode\n",
@@ -799,7 +799,7 @@ static int ap_sta_ps_end(struct sta_info *sta)
 
         atomic_dec(&sdata->bss->num_sta_ps);
 
-        clear_sta_flags(sta, WLAN_STA_PS | WLAN_STA_PSPOLL);
+        clear_sta_flags(sta, WLAN_STA_PS);
         drv_sta_notify(local, &sdata->vif, STA_NOTIFY_AWAKE, &sta->sta);
 
         if (!skb_queue_empty(&sta->ps_tx_buf))
@@ -1117,14 +1117,15 @@ ieee80211_rx_h_ps_poll(struct ieee80211_rx_data *rx)
                 skb_queue_empty(&rx->sta->ps_tx_buf);
 
         if (skb) {
+                struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
                 struct ieee80211_hdr *hdr =
                         (struct ieee80211_hdr *) skb->data;
 
                 /*
-                 * Tell TX path to send one frame even though the STA may
+                 * Tell TX path to send this frame even though the STA may
                  * still remain is PS mode after this frame exchange.
                  */
-                set_sta_flags(rx->sta, WLAN_STA_PSPOLL);
+                info->flags |= IEEE80211_TX_CTL_PSPOLL_RESPONSE;
 
 #ifdef CONFIG_MAC80211_VERBOSE_PS_DEBUG
                 printk(KERN_DEBUG "STA %pM aid %d: PS Poll (entries after %d)\n",
@@ -1139,7 +1140,7 @@ ieee80211_rx_h_ps_poll(struct ieee80211_rx_data *rx)
                 else
                         hdr->frame_control |= cpu_to_le16(IEEE80211_FCTL_MOREDATA);
 
-                dev_queue_xmit(skb);
+                ieee80211_add_pending_skb(rx->local, skb);
 
                 if (no_pending_pkts)
                         sta_info_clear_tim_bit(rx->sta);
diff --git a/net/mac80211/sta_info.h b/net/mac80211/sta_info.h
index 4ecf10a9bd00..ccc3adf962c7 100644
--- a/net/mac80211/sta_info.h
+++ b/net/mac80211/sta_info.h
@@ -30,7 +30,6 @@
  * @WLAN_STA_ASSOC_AP: We're associated to that station, it is an AP.
  * @WLAN_STA_WME: Station is a QoS-STA.
  * @WLAN_STA_WDS: Station is one of our WDS peers.
- * @WLAN_STA_PSPOLL: Station has just PS-polled us.
  * @WLAN_STA_CLEAR_PS_FILT: Clear PS filter in hardware (using the
  *      IEEE80211_TX_CTL_CLEAR_PS_FILT control flag) when the next
  *      frame to this station is transmitted.
@@ -47,7 +46,6 @@ enum ieee80211_sta_info_flags {
         WLAN_STA_ASSOC_AP       = 1<<5,
         WLAN_STA_WME            = 1<<6,
         WLAN_STA_WDS            = 1<<7,
-        WLAN_STA_PSPOLL         = 1<<8,
         WLAN_STA_CLEAR_PS_FILT  = 1<<9,
         WLAN_STA_MFP            = 1<<10,
         WLAN_STA_SUSPEND        = 1<<11
@@ -359,17 +357,6 @@ static inline void clear_sta_flags(struct sta_info *sta, const u32 flags)
         spin_unlock_irqrestore(&sta->flaglock, irqfl);
 }
 
-static inline void set_and_clear_sta_flags(struct sta_info *sta,
-                                           const u32 set, const u32 clear)
-{
-        unsigned long irqfl;
-
-        spin_lock_irqsave(&sta->flaglock, irqfl);
-        sta->flags |= set;
-        sta->flags &= ~clear;
-        spin_unlock_irqrestore(&sta->flaglock, irqfl);
-}
-
 static inline u32 test_sta_flags(struct sta_info *sta, const u32 flags)
 {
         u32 ret;
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 70ff4f065665..edacad1fb1dc 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -373,7 +373,7 @@ ieee80211_tx_h_unicast_ps_buf(struct ieee80211_tx_data *tx)
         staflags = get_sta_flags(sta);
 
         if (unlikely((staflags & WLAN_STA_PS) &&
-                     !(staflags & WLAN_STA_PSPOLL))) {
+                     !(info->flags & IEEE80211_TX_CTL_PSPOLL_RESPONSE))) {
 #ifdef CONFIG_MAC80211_VERBOSE_PS_DEBUG
                 printk(KERN_DEBUG "STA %pM aid %d: PS buffer (entries "
                        "before %d)\n",
@@ -412,24 +412,7 @@ ieee80211_tx_h_unicast_ps_buf(struct ieee80211_tx_data *tx)
                        sta->sta.addr);
         }
 #endif /* CONFIG_MAC80211_VERBOSE_PS_DEBUG */
-        if (test_and_clear_sta_flags(sta, WLAN_STA_PSPOLL)) {
-                /*
-                 * The sleeping station with pending data is now snoozing.
-                 * It queried us for its buffered frames and will go back
-                 * to deep sleep once it got everything.
-                 *
-                 * inform the driver, in case the hardware does powersave
-                 * frame filtering and keeps a station  blacklist on its own
-                 * (e.g: p54), so that frames can be delivered unimpeded.
-                 *
-                 * Note: It should be safe to disable the filter now.
-                 * As, it is really unlikely that we still have any pending
-                 * frame for this station in the hw's buffers/fifos left,
-                 * that is not rejected with a unsuccessful tx_status yet.
-                 */
 
-                info->flags |= IEEE80211_TX_CTL_CLEAR_PS_FILT;
-        }
         return TX_CONTINUE;
 }