mac80211: fix TKIP races, make API easier to use

Our current TKIP code races against itself on TX
since we can process multiple packets at the same
time on different ACs, but they all share the TX
context for TKIP. This can lead to bad IVs etc.

Also, the crypto offload helper code just obtains
the P1K/P2K from the cache, and can update it as
well, but there's no guarantee that packets are
really processed in order.

To fix these issues, first introduce a spinlock
that will protect the IV16/IV32 values in the TX
context. This first step makes sure that we don't
assign the same IV multiple times or get confused
in other ways.

Secondly, change the way the P1K cache works. I
add a field "p1k_iv32" that stores the value of
the IV32 when the P1K was last recomputed, and
if different from the last time, then a new P1K
is recomputed. This can cause the P1K computation
to flip back and forth if packets are processed
out of order. All this also happens under the new
spinlock.

Finally, because there are argument differences,
split up the ieee80211_get_tkip_key() API into
ieee80211_get_tkip_p1k() and ieee80211_get_tkip_p2k()
and give them the correct arguments.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
Signed-off-by: John W. Linville <linville@tuxdriver.com>

5 files changed, 78 insertions, 61 deletions

diff --git a/net/mac80211/key.c b/net/mac80211/key.c
index 1208a78..d930d4d 100644
--- a/net/mac80211/key.c
+++ b/net/mac80211/key.c
@@ -369,6 +369,7 @@ struct ieee80211_key *ieee80211_key_alloc(u32 cipher, int idx, size_t key_len,
 					get_unaligned_le16(seq);
 			}
 		}
+		spin_lock_init(&key->u.tkip.txlock);
 		break;
 	case WLAN_CIPHER_SUITE_CCMP:
 		key->conf.iv_len = CCMP_HDR_LEN;
diff --git a/net/mac80211/key.h b/net/mac80211/key.h
index d801d53..1493c3e 100644
--- a/net/mac80211/key.h
+++ b/net/mac80211/key.h
@@ -52,9 +52,10 @@ enum ieee80211_internal_tkip_state {
 };
 
 struct tkip_ctx {
-	u32 iv32;
-	u16 iv16;
-	u16 p1k[5];
+	u32 iv32;	/* current iv32 */
+	u16 iv16;	/* current iv16 */
+	u16 p1k[5];	/* p1k cache */
+	u32 p1k_iv32;	/* iv32 for which p1k computed */
 	enum ieee80211_internal_tkip_state state;
 };
 
@@ -71,6 +72,9 @@ struct ieee80211_key {
 
 	union {
 		struct {
+			/* protects tx context */
+			spinlock_t txlock;
+
 			/* last used TSC */
 			struct tkip_ctx tx;
 
diff --git a/net/mac80211/tkip.c b/net/mac80211/tkip.c
index 757e4eb..de570b3 100644
--- a/net/mac80211/tkip.c
+++ b/net/mac80211/tkip.c
@@ -101,6 +101,7 @@ static void tkip_mixing_phase1(const u8 *tk, struct tkip_ctx *ctx,
 		p1k[4] += tkipS(p1k[3] ^ get_unaligned_le16(tk + 0 + j)) + i;
 	}
 	ctx->state = TKIP_STATE_PHASE1_DONE;
+	ctx->p1k_iv32 = tsc_IV32;
 }
 
 static void tkip_mixing_phase2(const u8 *tk, struct tkip_ctx *ctx,
@@ -140,60 +141,72 @@ static void tkip_mixing_phase2(const u8 *tk, struct tkip_ctx *ctx,
 /* Add TKIP IV and Ext. IV at @pos. @iv0, @iv1, and @iv2 are the first octets
  * of the IV. Returns pointer to the octet following IVs (i.e., beginning of
  * the packet payload). */
-u8 *ieee80211_tkip_add_iv(u8 *pos, struct ieee80211_key *key, u16 iv16)
+u8 *ieee80211_tkip_add_iv(u8 *pos, struct ieee80211_key *key)
 {
-	pos = write_tkip_iv(pos, iv16);
+	lockdep_assert_held(&key->u.tkip.txlock);
+
+	pos = write_tkip_iv(pos, key->u.tkip.tx.iv16);
 	*pos++ = (key->conf.keyidx << 6) | (1 << 5) /* Ext IV */;
 	put_unaligned_le32(key->u.tkip.tx.iv32, pos);
 	return pos + 4;
 }
 
-void ieee80211_get_tkip_key(struct ieee80211_key_conf *keyconf,
-			struct sk_buff *skb, enum ieee80211_tkip_key_type type,
-			u8 *outkey)
+static void ieee80211_compute_tkip_p1k(struct ieee80211_key *key, u32 iv32)
+{
+	struct ieee80211_sub_if_data *sdata = key->sdata;
+	struct tkip_ctx *ctx = &key->u.tkip.tx;
+	const u8 *tk = &key->conf.key[NL80211_TKIP_DATA_OFFSET_ENCR_KEY];
+
+	lockdep_assert_held(&key->u.tkip.txlock);
+
+	/*
+	 * Update the P1K when the IV32 is different from the value it
+	 * had when we last computed it (or when not initialised yet).
+	 * This might flip-flop back and forth if packets are processed
+	 * out-of-order due to the different ACs, but then we have to
+	 * just compute the P1K more often.
+	 */
+	if (ctx->p1k_iv32 != iv32 || ctx->state == TKIP_STATE_NOT_INIT)
+		tkip_mixing_phase1(tk, ctx, sdata->vif.addr, iv32);
+}
+
+void ieee80211_get_tkip_p1k(struct ieee80211_key_conf *keyconf,
+			    struct sk_buff *skb, u16 *p1k)
 {
 	struct ieee80211_key *key = (struct ieee80211_key *)
 			container_of(keyconf, struct ieee80211_key, conf);
+	struct tkip_ctx *ctx = &key->u.tkip.tx;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
-	u8 *data;
-	const u8 *tk;
-	struct tkip_ctx *ctx;
-	u16 iv16;
-	u32 iv32;
-
-	data = (u8 *)hdr + ieee80211_hdrlen(hdr->frame_control);
-	iv16 = data[2] | (data[0] << 8);
-	iv32 = get_unaligned_le32(&data[4]);
-
-	tk = &key->conf.key[NL80211_TKIP_DATA_OFFSET_ENCR_KEY];
-	ctx = &key->u.tkip.tx;
-
-#ifdef CONFIG_MAC80211_TKIP_DEBUG
-	printk(KERN_DEBUG "TKIP encrypt: iv16 = 0x%04x, iv32 = 0x%08x\n",
-			iv16, iv32);
-
-	if (iv32 != ctx->iv32) {
-		printk(KERN_DEBUG "skb: iv32 = 0x%08x key: iv32 = 0x%08x\n",
-			iv32, ctx->iv32);
-		printk(KERN_DEBUG "Wrap around of iv16 in the middle of a "
-			"fragmented packet\n");
-	}
-#endif
-
-	/* Update the p1k only when the iv16 in the packet wraps around, this
-	 * might occur after the wrap around of iv16 in the key in case of
-	 * fragmented packets. */
-	if (iv16 == 0 || ctx->state == TKIP_STATE_NOT_INIT)
-		tkip_mixing_phase1(tk, ctx, hdr->addr2, iv32);
-
-	if (type == IEEE80211_TKIP_P1_KEY) {
-		memcpy(outkey, ctx->p1k, sizeof(u16) * 5);
-		return;
-	}
+	const u8 *data = (u8 *)hdr + ieee80211_hdrlen(hdr->frame_control);
+	u32 iv32 = get_unaligned_le32(&data[4]);
+	unsigned long flags;
+
+	spin_lock_irqsave(&key->u.tkip.txlock, flags);
+	ieee80211_compute_tkip_p1k(key, iv32);
+	memcpy(p1k, ctx->p1k, sizeof(ctx->p1k));
+	spin_unlock_irqrestore(&key->u.tkip.txlock, flags);
+}
+EXPORT_SYMBOL(ieee80211_get_tkip_p1k);
 
-	tkip_mixing_phase2(tk, ctx, iv16, outkey);
+void ieee80211_get_tkip_p2k(struct ieee80211_key_conf *keyconf,
+			    struct sk_buff *skb, u8 *p2k)
+{
+	struct ieee80211_key *key = (struct ieee80211_key *)
+			container_of(keyconf, struct ieee80211_key, conf);
+	const u8 *tk = &key->conf.key[NL80211_TKIP_DATA_OFFSET_ENCR_KEY];
+	struct tkip_ctx *ctx = &key->u.tkip.tx;
+	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+	const u8 *data = (u8 *)hdr + ieee80211_hdrlen(hdr->frame_control);
+	u32 iv32 = get_unaligned_le32(&data[4]);
+	u16 iv16 = data[2] | (data[0] << 8);
+	unsigned long flags;
+
+	spin_lock_irqsave(&key->u.tkip.txlock, flags);
+	ieee80211_compute_tkip_p1k(key, iv32);
+	tkip_mixing_phase2(tk, ctx, iv16, p2k);
+	spin_unlock_irqrestore(&key->u.tkip.txlock, flags);
 }
-EXPORT_SYMBOL(ieee80211_get_tkip_key);
+EXPORT_SYMBOL(ieee80211_get_tkip_p2k);
 
 /*
  * Encrypt packet payload with TKIP using @key. @pos is a pointer to the
@@ -204,19 +217,15 @@ EXPORT_SYMBOL(ieee80211_get_tkip_key);
  */
 int ieee80211_tkip_encrypt_data(struct crypto_cipher *tfm,
 				struct ieee80211_key *key,
-				u8 *pos, size_t payload_len, u8 *ta)
+				struct sk_buff *skb,
+				u8 *payload, size_t payload_len)
 {
 	u8 rc4key[16];
-	struct tkip_ctx *ctx = &key->u.tkip.tx;
-	const u8 *tk = &key->conf.key[NL80211_TKIP_DATA_OFFSET_ENCR_KEY];
-
-	/* Calculate per-packet key */
-	if (ctx->iv16 == 0 || ctx->state == TKIP_STATE_NOT_INIT)
-		tkip_mixing_phase1(tk, ctx, ta, ctx->iv32);
 
-	tkip_mixing_phase2(tk, ctx, ctx->iv16, rc4key);
+	ieee80211_get_tkip_p2k(&key->conf, skb, rc4key);
 
-	return ieee80211_wep_encrypt_data(tfm, rc4key, 16, pos, payload_len);
+	return ieee80211_wep_encrypt_data(tfm, rc4key, 16,
+					  payload, payload_len);
 }
 
 /* Decrypt packet payload with TKIP using @key. @pos is a pointer to the
diff --git a/net/mac80211/tkip.h b/net/mac80211/tkip.h
index 1cab9c8..e3ecb65 100644
--- a/net/mac80211/tkip.h
+++ b/net/mac80211/tkip.h
@@ -13,11 +13,13 @@
 #include <linux/crypto.h>
 #include "key.h"
 
-u8 *ieee80211_tkip_add_iv(u8 *pos, struct ieee80211_key *key, u16 iv16);
+u8 *ieee80211_tkip_add_iv(u8 *pos, struct ieee80211_key *key);
 
 int ieee80211_tkip_encrypt_data(struct crypto_cipher *tfm,
-				 struct ieee80211_key *key,
-				 u8 *pos, size_t payload_len, u8 *ta);
+				struct ieee80211_key *key,
+				struct sk_buff *skb,
+				u8 *payload, size_t payload_len);
+
 enum {
 	TKIP_DECRYPT_OK = 0,
 	TKIP_DECRYPT_NO_EXT_IV = -1,
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index d91c1a2..4ded2ae 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -171,6 +171,7 @@ static int tkip_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
 	struct ieee80211_key *key = tx->key;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
+	unsigned long flags;
 	unsigned int hdrlen;
 	int len, tail;
 	u8 *pos;
@@ -198,11 +199,12 @@ static int tkip_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
 	pos += hdrlen;
 
 	/* Increase IV for the frame */
+	spin_lock_irqsave(&key->u.tkip.txlock, flags);
 	key->u.tkip.tx.iv16++;
 	if (key->u.tkip.tx.iv16 == 0)
 		key->u.tkip.tx.iv32++;
-
-	pos = ieee80211_tkip_add_iv(pos, key, key->u.tkip.tx.iv16);
+	pos = ieee80211_tkip_add_iv(pos, key);
+	spin_unlock_irqrestore(&key->u.tkip.txlock, flags);
 
 	/* hwaccel - with software IV */
 	if (info->control.hw_key)
@@ -211,9 +213,8 @@ static int tkip_encrypt_skb(struct ieee80211_tx_data *tx, struct sk_buff *skb)
 	/* Add room for ICV */
 	skb_put(skb, TKIP_ICV_LEN);
 
-	hdr = (struct ieee80211_hdr *) skb->data;
 	return ieee80211_tkip_encrypt_data(tx->local->wep_tx_tfm,
-					   key, pos, len, hdr->addr2);
+					   key, skb, pos, len);
 }