linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
To: johannes@sipsolutions.net
Cc: linux-wireless@vger.kernel.org,
	Emmanuel Grumbach <emmanuel.grumbach@intel.com>,
	stable@vger.kernel.org
Subject: [PATCH] mac80211: check if the ps_buf skb queue hasn't been drained
Date: Thu, 20 Feb 2014 08:18:37 +0200	[thread overview]
Message-ID: <1392877117-17869-1-git-send-email-emmanuel.grumbach@intel.com> (raw)

There is a race between the Tx path and the STA wake up
flow.
If a station is asleep, mac80211 buffers frames, but up to
a certain limit. When this limit is reached, mac80211 begins
to drop the oldest buffered frames. This is done in the Tx
path.
When a station wakes up, mac80211 will go over the buffered
frames list and send them.
Since these two flows can run concurrently, we can get to a
situation where the buffered frame list is emptied after the
Tx path checked the length of the list. In that case the Tx
path would get a NULL skb and crash.

This is only a noisy consequence of another bigger issue:
since ieee80211_tx_h_unicast_ps_buf is not synced against
ieee80211_sta_ps_deliver_wakeup, Tx path could think that
the STA is still sleeping and put the packet on the PS skb
list while in fact the STA is awake already. In this case,
the frame would sit on the PS skb list until the next time
the STA wakes up and then, it'd be sent out of order.

Fix all this by syncing both path with sta_info->lock.
Both path are not fastpath since they handle PS situations.

This fixes:

BUG: unable to handle kernel
NULL pointer dereference at 000000b0
IP: [<ff6f1791>] ieee80211_report_used_skb+0x11/0x3e0 [mac80211]
*pde = 00000000
Oops: 0000 [#1] SMP DEBUG_PAGEALLOC
EIP: 0060:[<ff6f1791>] EFLAGS: 00210282 CPU: 1
EIP is at ieee80211_report_used_skb+0x11/0x3e0 [mac80211]
EAX: e5900da0 EBX: 00000000 ECX: 00000001 EDX: 00000000
ESI: e41d00c0 EDI: e5900da0 EBP: ebe458e4 ESP: ebe458b0
 DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
CR0: 8005003b CR2: 000000b0 CR3: 25a78000 CR4: 000407d0
DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
DR6: ffff0ff0 DR7: 00000400
Process iperf (pid: 3934, ti=ebe44000 task=e757c0b0 task.ti=ebe44000)
iwlwifi 0000:02:00.0: I iwl_pcie_enqueue_hcmd Sending command LQ_CMD (#4e), seq: 0x0903, 92 bytes at 3[3]:9
Stack:
 e403b32c ebe458c4 00200002 00200286 e403b338 ebe458cc c10960bb e5900da0
 ff76a6ec ebe458d8 00000000 e41d00c0 e5900da0 ebe458f0 ff6f1b75 e403b210
 ebe4598c ff723dc1 00000000 ff76a6ec e597c978 e403b758 00000002 00000002
Call Trace:
 [<c10960bb>] ? trace_hardirqs_on+0xb/0x10
 [<ff6f1b75>] ieee80211_free_txskb+0x15/0x20 [mac80211]
 [<ff723dc1>] invoke_tx_handlers+0x1661/0x1780 [mac80211]
 [<ff7243c7>] ? ieee80211_tx_prepare+0x4e7/0x550 [mac80211]
 [<ff7248a5>] ieee80211_tx+0x75/0x100 [mac80211]
 [<ff7249bf>] ieee80211_xmit+0x8f/0xc0 [mac80211]
 [<ff72550e>] ieee80211_subif_start_xmit+0x4fe/0xe20 [mac80211]
 [<ff725044>] ? ieee80211_subif_start_xmit+0x34/0xe20 [mac80211]
 [<c149ef70>] dev_hard_start_xmit+0x450/0x950
 [<c14b9a8a>] ? sch_direct_xmit+0x8a/0x250
 [<c14b9a8a>] ? sch_direct_xmit+0x8a/0x250
 [<c15afa5c>] ? _raw_spin_lock+0x6c/0x80
 [<c14b9aa9>] sch_direct_xmit+0xa9/0x250
 [<c149f5ab>] ? dev_queue_xmit+0x13b/0xca0
 [<c149f5ab>] ? dev_queue_xmit+0x13b/0xca0
 [<c14b9c9b>] __qdisc_run+0x4b/0x150
 [<c149f732>] dev_queue_xmit+0x2c2/0xca0

[snip]

Code: e8 65 56 bc fe 83 c4 04 5b 5d c3 eb 0d 90 90 90 90 90 90 90 90 90 90 90 90 90 55 89 e5 57 56 53 83 ec 28 66 66 66 66 90 89 45 e8 <8b> 82 b0 00 00 00 84 c9 89 d3 89 ce c6 45 ef 00 89 45 e4 8b 42
EIP: [<ff6f1791>] ieee80211_report_used_skb+0x11/0x3e0 [mac80211] SS:ESP 0068:ebe458b0
CR2: 00000000000000b0

Cc: stable@vger.kernel.org
Reported-by: Yaara Rozenblum <yaara.rozenblum@intel.com>
Signed-off-by: Emmanuel Grumbach <emmanuel.grumbach@intel.com>
---
 net/mac80211/sta_info.c |  3 +++
 net/mac80211/tx.c       | 14 ++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index f9ce6ff..281ec1f 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1099,6 +1099,8 @@ void ieee80211_sta_ps_deliver_wakeup(struct sta_info *sta)
 
 	skb_queue_head_init(&pending);
 
+	/* sync with ieee80211_tx_h_unicast_ps_buf */
+	spin_lock(&sta->lock);
 	/* Send all buffered frames to the station */
 	for (ac = 0; ac < IEEE80211_NUM_ACS; ac++) {
 		int count = skb_queue_len(&pending), tmp;
@@ -1118,6 +1120,7 @@ void ieee80211_sta_ps_deliver_wakeup(struct sta_info *sta)
 	}
 
 	ieee80211_add_pending_skbs_fn(local, &pending, clear_sta_ps_flags, sta);
+	spin_unlock(&sta->lock);
 
 	/* This station just woke up and isn't aware of our SMPS state */
 	if (!ieee80211_smps_is_restrictive(sta->known_smps_mode,
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 94e0102..200515f 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -478,6 +478,19 @@ ieee80211_tx_h_unicast_ps_buf(struct ieee80211_tx_data *tx)
 		       sta->sta.addr, sta->sta.aid, ac);
 		if (tx->local->total_ps_buffered >= TOTAL_MAX_TX_BUFFER)
 			purge_old_ps_buffers(tx->local);
+
+		/* sync with ieee80211_sta_ps_deliver_wakeup */
+		spin_lock(&sta->lock);
+		/*
+		 * STA woke up the meantime and all the frames on ps_tx_buf have
+		 * been queued to pending queue. No reordering can happen, go
+		 * ahead and Tx the packet.
+		 */
+		if (!test_sta_flag(sta, WLAN_STA_PS_STA)) {
+			spin_unlock(&sta->lock);
+			return TX_CONTINUE;
+		}
+
 		if (skb_queue_len(&sta->ps_tx_buf[ac]) >= STA_MAX_TX_BUFFER) {
 			struct sk_buff *old = skb_dequeue(&sta->ps_tx_buf[ac]);
 			ps_dbg(tx->sdata,
@@ -486,6 +499,7 @@ ieee80211_tx_h_unicast_ps_buf(struct ieee80211_tx_data *tx)
 			ieee80211_free_txskb(&local->hw, old);
 		} else
 			tx->local->total_ps_buffered++;
+		spin_unlock(&sta->lock);
 
 		info->control.jiffies = jiffies;
 		info->control.vif = &tx->sdata->vif;
-- 
1.8.3.2


             reply	other threads:[~2014-02-20  6:20 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-02-20  6:18 Emmanuel Grumbach [this message]
2014-02-20  7:07 ` [PATCH] mac80211: check if the ps_buf skb queue hasn't been drained Johannes Berg
2014-02-20  7:22 ` [PATCH v2] " Emmanuel Grumbach
2014-02-20  8:26   ` Johannes Berg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1392877117-17869-1-git-send-email-emmanuel.grumbach@intel.com \
    --to=emmanuel.grumbach@intel.com \
    --cc=johannes@sipsolutions.net \
    --cc=linux-wireless@vger.kernel.org \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).