[PATCH 0/2] fix mac80211 crypto races

[PATCH 0/2] fix mac80211 crypto races

[Johannes Berg]

mac80211's crypto code is racing against itself, since
we can process multiple TX packets at the same time on
different ACs.

These two patches fix this. I'm not backporting them
since nobody complained, and the worst case probably
is lost packets (though in the case of TKIP maybe it
can cause countermeasures?)

johannes

[PATCH 1/2] mac80211: fix TKIP races, make API easier to use

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

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>
---
 drivers/net/wireless/b43/xmit.c             |    3 
 drivers/net/wireless/iwlegacy/iwl-4965-tx.c |    3 
 drivers/net/wireless/iwlwifi/iwl-agn-tx.c   |    3 
 include/net/mac80211.h                      |   51 ++++++------
 net/mac80211/key.c                          |    1 
 net/mac80211/key.h                          |   10 +-
 net/mac80211/tkip.c                         |  111 +++++++++++++++-------------
 net/mac80211/tkip.h                         |    8 +-
 net/mac80211/wpa.c                          |    9 +-
 9 files changed, 105 insertions(+), 94 deletions(-)

--- a/include/net/mac80211.h	2011-07-06 12:08:39.000000000 +0200
+++ b/include/net/mac80211.h	2011-07-06 12:08:57.000000000 +0200
@@ -962,21 +962,6 @@ enum sta_notify_cmd {
 };
 
 /**
- * enum ieee80211_tkip_key_type - get tkip key
- *
- * Used by drivers which need to get a tkip key for skb. Some drivers need a
- * phase 1 key, others need a phase 2 key. A single function allows the driver
- * to get the key, this enum indicates what type of key is required.
- *
- * @IEEE80211_TKIP_P1_KEY: the driver needs a phase 1 key
- * @IEEE80211_TKIP_P2_KEY: the driver needs a phase 2 key
- */
-enum ieee80211_tkip_key_type {
-	IEEE80211_TKIP_P1_KEY,
-	IEEE80211_TKIP_P2_KEY,
-};
-
-/**
  * enum ieee80211_hw_flags - hardware flags
  *
  * These flags are used to indicate hardware capabilities to
@@ -2579,21 +2564,33 @@ struct sk_buff *
 ieee80211_get_buffered_bc(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
 
 /**
- * ieee80211_get_tkip_key - get a TKIP rc4 for skb
+ * ieee80211_get_tkip_p1k - get a TKIP phase 1 key
+ *
+ * This function returns the TKIP phase 1 key for the IV32 taken
+ * from the given packet.
+ *
+ * @keyconf: the parameter passed with the set key
+ * @skb: the packet to take the IV32 value from that will be encrypted
+ *	with this P1K
+ * @p1k: a buffer to which the key will be written, as 5 u16 values
+ */
+void ieee80211_get_tkip_p1k(struct ieee80211_key_conf *keyconf,
+			    struct sk_buff *skb, u16 *p1k);
+
+/**
+ * ieee80211_get_tkip_p2k - get a TKIP phase 2 key
  *
- * This function computes a TKIP rc4 key for an skb. It computes
- * a phase 1 key if needed (iv16 wraps around). This function is to
- * be used by drivers which can do HW encryption but need to compute
- * to phase 1/2 key in SW.
+ * This function computes the TKIP RC4 key for the IV values
+ * in the packet.
  *
  * @keyconf: the parameter passed with the set key
- * @skb: the skb for which the key is needed
- * @type: TBD
- * @key: a buffer to which the key will be written
- */
-void ieee80211_get_tkip_key(struct ieee80211_key_conf *keyconf,
-				struct sk_buff *skb,
-				enum ieee80211_tkip_key_type type, u8 *key);
+ * @skb: the packet to take the IV32/IV16 values from that will be
+ *	encrypted with this key
+ * @p2k: a buffer to which the key will be written, 16 bytes
+ */
+void ieee80211_get_tkip_p2k(struct ieee80211_key_conf *keyconf,
+			    struct sk_buff *skb, u8 *p2k);
+
 /**
  * ieee80211_gtk_rekey_notify - notify userspace supplicant of rekeying
  * @vif: virtual interface the rekeying was done on
--- a/net/mac80211/tkip.c	2011-07-06 12:06:57.000000000 +0200
+++ b/net/mac80211/tkip.c	2011-07-06 12:08:57.000000000 +0200
@@ -101,6 +101,7 @@ static void tkip_mixing_phase1(const u8
 		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
 /* 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
--- a/drivers/net/wireless/b43/xmit.c	2011-07-06 12:06:57.000000000 +0200
+++ b/drivers/net/wireless/b43/xmit.c	2011-07-06 12:08:57.000000000 +0200
@@ -323,8 +323,7 @@ int b43_generate_txhdr(struct b43_wldev
 			/* we give the phase1key and iv16 here, the key is stored in
 			 * shm. With that the hardware can do phase 2 and encryption.
 			 */
-			ieee80211_get_tkip_key(info->control.hw_key, skb_frag,
-					IEEE80211_TKIP_P1_KEY, (u8*)phase1key);
+			ieee80211_get_tkip_p1k(info->control.hw_key, skb_frag, phase1key);
 			/* phase1key is in host endian. Copy to little-endian txhdr->iv. */
 			for (i = 0; i < 5; i++) {
 				txhdr->iv[i * 2 + 0] = phase1key[i];
--- a/drivers/net/wireless/iwlegacy/iwl-4965-tx.c	2011-07-06 12:06:57.000000000 +0200
+++ b/drivers/net/wireless/iwlegacy/iwl-4965-tx.c	2011-07-06 12:08:57.000000000 +0200
@@ -240,8 +240,7 @@ static void iwl4965_tx_cmd_build_hwcrypt
 
 	case WLAN_CIPHER_SUITE_TKIP:
 		tx_cmd->sec_ctl = TX_CMD_SEC_TKIP;
-		ieee80211_get_tkip_key(keyconf, skb_frag,
-			IEEE80211_TKIP_P2_KEY, tx_cmd->key);
+		ieee80211_get_tkip_p2k(keyconf, skb_frag, tx_cmd->key);
 		IWL_DEBUG_TX(priv, "tx_cmd with tkip hwcrypto\n");
 		break;
 
--- a/drivers/net/wireless/iwlwifi/iwl-agn-tx.c	2011-07-06 12:07:43.000000000 +0200
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-tx.c	2011-07-06 12:08:57.000000000 +0200
@@ -497,8 +497,7 @@ static void iwlagn_tx_cmd_build_hwcrypto
 
 	case WLAN_CIPHER_SUITE_TKIP:
 		tx_cmd->sec_ctl = TX_CMD_SEC_TKIP;
-		ieee80211_get_tkip_key(keyconf, skb_frag,
-			IEEE80211_TKIP_P2_KEY, tx_cmd->key);
+		ieee80211_get_tkip_p2k(keyconf, skb_frag, tx_cmd->key);
 		IWL_DEBUG_TX(priv, "tx_cmd with tkip hwcrypto\n");
 		break;
 
--- a/net/mac80211/key.c	2011-07-06 12:08:39.000000000 +0200
+++ b/net/mac80211/key.c	2011-07-06 12:08:57.000000000 +0200
@@ -369,6 +369,7 @@ struct ieee80211_key *ieee80211_key_allo
 					get_unaligned_le16(seq);
 			}
 		}
+		spin_lock_init(&key->u.tkip.txlock);
 		break;
 	case WLAN_CIPHER_SUITE_CCMP:
 		key->conf.iv_len = CCMP_HDR_LEN;
--- a/net/mac80211/key.h	2011-07-06 12:06:57.000000000 +0200
+++ b/net/mac80211/key.h	2011-07-06 12:08:57.000000000 +0200
@@ -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;
 
--- a/net/mac80211/tkip.h	2011-07-06 12:06:57.000000000 +0200
+++ b/net/mac80211/tkip.h	2011-07-06 12:08:57.000000000 +0200
@@ -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,
--- a/net/mac80211/wpa.c	2011-07-06 12:07:45.000000000 +0200
+++ b/net/mac80211/wpa.c	2011-07-06 12:08:57.000000000 +0200
@@ -171,6 +171,7 @@ static int tkip_encrypt_skb(struct ieee8
 	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 ieee8
 	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 ieee8
 	/* 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);
 }
 
 

Re: [PATCH 1/2 v2] mac80211: fix TKIP races, make API easier to use

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

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>
---
v2: some whitespace oddities caused this to no longer apply.

 drivers/net/wireless/b43/xmit.c             |    3 
 drivers/net/wireless/iwlegacy/iwl-4965-tx.c |    3 
 drivers/net/wireless/iwlwifi/iwl-agn-tx.c   |    3 
 include/net/mac80211.h                      |   50 +++++-------
 net/mac80211/key.c                          |    1 
 net/mac80211/key.h                          |   10 +-
 net/mac80211/tkip.c                         |  111 +++++++++++++++-------------
 net/mac80211/tkip.h                         |    8 +-
 net/mac80211/wpa.c                          |    9 +-
 9 files changed, 104 insertions(+), 94 deletions(-)

--- a/include/net/mac80211.h	2011-07-07 22:26:40.000000000 +0200
+++ b/include/net/mac80211.h	2011-07-07 22:26:49.000000000 +0200
@@ -973,21 +973,6 @@ enum sta_notify_cmd {
 };
 
 /**
- * enum ieee80211_tkip_key_type - get tkip key
- *
- * Used by drivers which need to get a tkip key for skb. Some drivers need a
- * phase 1 key, others need a phase 2 key. A single function allows the driver
- * to get the key, this enum indicates what type of key is required.
- *
- * @IEEE80211_TKIP_P1_KEY: the driver needs a phase 1 key
- * @IEEE80211_TKIP_P2_KEY: the driver needs a phase 2 key
- */
-enum ieee80211_tkip_key_type {
-	IEEE80211_TKIP_P1_KEY,
-	IEEE80211_TKIP_P2_KEY,
-};
-
-/**
  * enum ieee80211_hw_flags - hardware flags
  *
  * These flags are used to indicate hardware capabilities to
@@ -2594,21 +2579,32 @@ struct sk_buff *
 ieee80211_get_buffered_bc(struct ieee80211_hw *hw, struct ieee80211_vif *vif);
 
 /**
- * ieee80211_get_tkip_key - get a TKIP rc4 for skb
+ * ieee80211_get_tkip_p1k - get a TKIP phase 1 key
  *
- * This function computes a TKIP rc4 key for an skb. It computes
- * a phase 1 key if needed (iv16 wraps around). This function is to
- * be used by drivers which can do HW encryption but need to compute
- * to phase 1/2 key in SW.
+ * This function returns the TKIP phase 1 key for the IV32 taken
+ * from the given packet.
  *
  * @keyconf: the parameter passed with the set key
- * @skb: the skb for which the key is needed
- * @type: TBD
- * @key: a buffer to which the key will be written
- */
-void ieee80211_get_tkip_key(struct ieee80211_key_conf *keyconf,
-				struct sk_buff *skb,
-				enum ieee80211_tkip_key_type type, u8 *key);
+ * @skb: the packet to take the IV32 value from that will be encrypted
+ *	with this P1K
+ * @p1k: a buffer to which the key will be written, as 5 u16 values
+ */
+void ieee80211_get_tkip_p1k(struct ieee80211_key_conf *keyconf,
+			    struct sk_buff *skb, u16 *p1k);
+
+/**
+ * ieee80211_get_tkip_p2k - get a TKIP phase 2 key
+ *
+ * This function computes the TKIP RC4 key for the IV values
+ * in the packet.
+ *
+ * @keyconf: the parameter passed with the set key
+ * @skb: the packet to take the IV32/IV16 values from that will be
+ *	encrypted with this key
+ * @p2k: a buffer to which the key will be written, 16 bytes
+ */
+void ieee80211_get_tkip_p2k(struct ieee80211_key_conf *keyconf,
+			    struct sk_buff *skb, u8 *p2k);
 
 /**
  * ieee80211_gtk_rekey_notify - notify userspace supplicant of rekeying
--- a/net/mac80211/tkip.c	2011-07-07 22:26:40.000000000 +0200
+++ b/net/mac80211/tkip.c	2011-07-07 22:26:49.000000000 +0200
@@ -101,6 +101,7 @@ static void tkip_mixing_phase1(const u8
 		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
 /* 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
--- a/drivers/net/wireless/b43/xmit.c	2011-07-07 22:26:40.000000000 +0200
+++ b/drivers/net/wireless/b43/xmit.c	2011-07-07 22:26:49.000000000 +0200
@@ -323,8 +323,7 @@ int b43_generate_txhdr(struct b43_wldev
 			/* we give the phase1key and iv16 here, the key is stored in
 			 * shm. With that the hardware can do phase 2 and encryption.
 			 */
-			ieee80211_get_tkip_key(info->control.hw_key, skb_frag,
-					IEEE80211_TKIP_P1_KEY, (u8*)phase1key);
+			ieee80211_get_tkip_p1k(info->control.hw_key, skb_frag, phase1key);
 			/* phase1key is in host endian. Copy to little-endian txhdr->iv. */
 			for (i = 0; i < 5; i++) {
 				txhdr->iv[i * 2 + 0] = phase1key[i];
--- a/drivers/net/wireless/iwlegacy/iwl-4965-tx.c	2011-07-07 22:26:40.000000000 +0200
+++ b/drivers/net/wireless/iwlegacy/iwl-4965-tx.c	2011-07-07 22:26:49.000000000 +0200
@@ -240,8 +240,7 @@ static void iwl4965_tx_cmd_build_hwcrypt
 
 	case WLAN_CIPHER_SUITE_TKIP:
 		tx_cmd->sec_ctl = TX_CMD_SEC_TKIP;
-		ieee80211_get_tkip_key(keyconf, skb_frag,
-			IEEE80211_TKIP_P2_KEY, tx_cmd->key);
+		ieee80211_get_tkip_p2k(keyconf, skb_frag, tx_cmd->key);
 		IWL_DEBUG_TX(priv, "tx_cmd with tkip hwcrypto\n");
 		break;
 
--- a/drivers/net/wireless/iwlwifi/iwl-agn-tx.c	2011-07-07 22:26:40.000000000 +0200
+++ b/drivers/net/wireless/iwlwifi/iwl-agn-tx.c	2011-07-07 22:26:49.000000000 +0200
@@ -514,8 +514,7 @@ static void iwlagn_tx_cmd_build_hwcrypto
 
 	case WLAN_CIPHER_SUITE_TKIP:
 		tx_cmd->sec_ctl = TX_CMD_SEC_TKIP;
-		ieee80211_get_tkip_key(keyconf, skb_frag,
-			IEEE80211_TKIP_P2_KEY, tx_cmd->key);
+		ieee80211_get_tkip_p2k(keyconf, skb_frag, tx_cmd->key);
 		IWL_DEBUG_TX(priv, "tx_cmd with tkip hwcrypto\n");
 		break;
 
--- a/net/mac80211/key.c	2011-07-07 22:26:40.000000000 +0200
+++ b/net/mac80211/key.c	2011-07-07 22:26:49.000000000 +0200
@@ -369,6 +369,7 @@ struct ieee80211_key *ieee80211_key_allo
 					get_unaligned_le16(seq);
 			}
 		}
+		spin_lock_init(&key->u.tkip.txlock);
 		break;
 	case WLAN_CIPHER_SUITE_CCMP:
 		key->conf.iv_len = CCMP_HDR_LEN;
--- a/net/mac80211/key.h	2011-07-07 22:26:40.000000000 +0200
+++ b/net/mac80211/key.h	2011-07-07 22:26:49.000000000 +0200
@@ -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;
 
--- a/net/mac80211/tkip.h	2011-07-07 22:26:40.000000000 +0200
+++ b/net/mac80211/tkip.h	2011-07-07 22:26:49.000000000 +0200
@@ -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,
--- a/net/mac80211/wpa.c	2011-07-07 22:26:48.000000000 +0200
+++ b/net/mac80211/wpa.c	2011-07-07 22:26:49.000000000 +0200
@@ -176,6 +176,7 @@ static int tkip_encrypt_skb(struct ieee8
 	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;
@@ -203,11 +204,12 @@ static int tkip_encrypt_skb(struct ieee8
 	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)
@@ -216,9 +218,8 @@ static int tkip_encrypt_skb(struct ieee8
 	/* 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);
 }
 
 

[PATCH 2/2] mac80211: fix CCMP PN race

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Since we can process multiple packets at the
same time for different ACs, but the PN is
allocated from a single counter, we need to
use an atomic value there. Use atomic64_t to
make this cheaper on 64-bit platforms, other
platforms will support this through software
emulation, see lib/atomic64.c.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/cfg.c         |   14 ++++++++------
 net/mac80211/debugfs_key.c |    6 ++++--
 net/mac80211/key.h         |    2 +-
 net/mac80211/wpa.c         |   19 ++++++++++---------
 4 files changed, 23 insertions(+), 18 deletions(-)

--- a/net/mac80211/key.h	2011-07-06 12:08:57.000000000 +0200
+++ b/net/mac80211/key.h	2011-07-06 12:09:07.000000000 +0200
@@ -82,7 +82,7 @@ struct ieee80211_key {
 			struct tkip_ctx rx[NUM_RX_DATA_QUEUES];
 		} tkip;
 		struct {
-			u8 tx_pn[6];
+			atomic64_t tx_pn;
 			/*
 			 * Last received packet number. The first
 			 * NUM_RX_DATA_QUEUES counters are used with Data
--- a/net/mac80211/cfg.c	2011-07-06 12:08:39.000000000 +0200
+++ b/net/mac80211/cfg.c	2011-07-06 12:09:07.000000000 +0200
@@ -209,6 +209,7 @@ static int ieee80211_get_key(struct wiph
 	u8 seq[6] = {0};
 	struct key_params params;
 	struct ieee80211_key *key = NULL;
+	u64 pn64;
 	u32 iv32;
 	u16 iv16;
 	int err = -ENOENT;
@@ -256,12 +257,13 @@ static int ieee80211_get_key(struct wiph
 		params.seq_len = 6;
 		break;
 	case WLAN_CIPHER_SUITE_CCMP:
-		seq[0] = key->u.ccmp.tx_pn[5];
-		seq[1] = key->u.ccmp.tx_pn[4];
-		seq[2] = key->u.ccmp.tx_pn[3];
-		seq[3] = key->u.ccmp.tx_pn[2];
-		seq[4] = key->u.ccmp.tx_pn[1];
-		seq[5] = key->u.ccmp.tx_pn[0];
+		pn64 = atomic64_read(&key->u.ccmp.tx_pn);
+		seq[0] = pn64;
+		seq[1] = pn64 >> 8;
+		seq[2] = pn64 >> 16;
+		seq[3] = pn64 >> 24;
+		seq[4] = pn64 >> 32;
+		seq[5] = pn64 >> 40;
 		params.seq = seq;
 		params.seq_len = 6;
 		break;
--- a/net/mac80211/wpa.c	2011-07-06 12:08:57.000000000 +0200
+++ b/net/mac80211/wpa.c	2011-07-06 12:09:07.000000000 +0200
@@ -380,8 +380,9 @@ static int ccmp_encrypt_skb(struct ieee8
 	struct ieee80211_key *key = tx->key;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	int hdrlen, len, tail;
-	u8 *pos, *pn;
-	int i;
+	u8 *pos;
+	u8 pn[6];
+	u64 pn64;
 
 	if (info->control.hw_key &&
 	    !(info->control.hw_key->flags & IEEE80211_KEY_FLAG_GENERATE_IV)) {
@@ -409,14 +410,14 @@ static int ccmp_encrypt_skb(struct ieee8
 	hdr = (struct ieee80211_hdr *) pos;
 	pos += hdrlen;
 
-	/* PN = PN + 1 */
-	pn = key->u.ccmp.tx_pn;
+	pn64 = atomic64_inc_return(&key->u.ccmp.tx_pn);
 
-	for (i = CCMP_PN_LEN - 1; i >= 0; i--) {
-		pn[i]++;
-		if (pn[i])
-			break;
-	}
+	pn[5] = pn64;
+	pn[4] = pn64 >> 8;
+	pn[3] = pn64 >> 16;
+	pn[2] = pn64 >> 24;
+	pn[1] = pn64 >> 32;
+	pn[0] = pn64 >> 40;
 
 	ccmp_pn2hdr(pos, pn, key->conf.keyidx);
 
--- a/net/mac80211/debugfs_key.c	2011-07-06 12:03:22.000000000 +0200
+++ b/net/mac80211/debugfs_key.c	2011-07-06 12:09:07.000000000 +0200
@@ -79,6 +79,7 @@ static ssize_t key_tx_spec_read(struct f
 				size_t count, loff_t *ppos)
 {
 	const u8 *tpn;
+	u64 pn;
 	char buf[20];
 	int len;
 	struct ieee80211_key *key = file->private_data;
@@ -94,9 +95,10 @@ static ssize_t key_tx_spec_read(struct f
 				key->u.tkip.tx.iv16);
 		break;
 	case WLAN_CIPHER_SUITE_CCMP:
-		tpn = key->u.ccmp.tx_pn;
+		pn = atomic64_read(&key->u.ccmp.tx_pn);
 		len = scnprintf(buf, sizeof(buf), "%02x%02x%02x%02x%02x%02x\n",
-				tpn[0], tpn[1], tpn[2], tpn[3], tpn[4], tpn[5]);
+				(u8)(pn >> 40), (u8)(pn >> 32), (u8)(pn >> 24),
+				(u8)(pn >> 16), (u8)(pn >> 8), (u8)pn);
 		break;
 	case WLAN_CIPHER_SUITE_AES_CMAC:
 		tpn = key->u.aes_cmac.tx_pn;

Re: [PATCH 2/2] mac80211: fix CCMP PN race

[Johannes Berg]

On Wed, 2011-07-06 at 12:09 +0200, Johannes Berg wrote:
> plain text document attachment (009-mac80211-fix-ccmp-races.patch)
> From: Johannes Berg <johannes.berg@intel.com>
> 
> Since we can process multiple packets at the
> same time for different ACs, but the PN is
> allocated from a single counter, we need to
> use an atomic value there. Use atomic64_t to
> make this cheaper on 64-bit platforms, other
> platforms will support this through software
> emulation, see lib/atomic64.c.

This fixes the PN race but is incomplete, will send a new version.

johannes

[PATCH 2/2 v2] mac80211: fix CCMP races

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Since we can process multiple packets at the
same time for different ACs, but the PN is
allocated from a single counter, we need to
use an atomic value there. Use atomic64_t to
make this cheaper on 64-bit platforms, other
platforms will support this through software
emulation, see lib/atomic64.c.

We also need to use an on-stack scratch buf
so that multiple packets won't corrupt each
others scratch buffers.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/cfg.c         |   14 ++++++++------
 net/mac80211/debugfs_key.c |    6 ++++--
 net/mac80211/key.h         |    5 +----
 net/mac80211/wpa.c         |   32 +++++++++++++++++++-------------
 4 files changed, 32 insertions(+), 25 deletions(-)

--- a/net/mac80211/key.h	2011-07-06 21:40:12.000000000 +0200
+++ b/net/mac80211/key.h	2011-07-06 21:59:19.000000000 +0200
@@ -82,7 +82,7 @@ struct ieee80211_key {
 			struct tkip_ctx rx[NUM_RX_DATA_QUEUES];
 		} tkip;
 		struct {
-			u8 tx_pn[6];
+			atomic64_t tx_pn;
 			/*
 			 * Last received packet number. The first
 			 * NUM_RX_DATA_QUEUES counters are used with Data
@@ -92,12 +92,9 @@ struct ieee80211_key {
 			u8 rx_pn[NUM_RX_DATA_QUEUES + 1][6];
 			struct crypto_cipher *tfm;
 			u32 replays; /* dot11RSNAStatsCCMPReplays */
-			/* scratch buffers for virt_to_page() (crypto API) */
 #ifndef AES_BLOCK_LEN
 #define AES_BLOCK_LEN 16
 #endif
-			u8 tx_crypto_buf[6 * AES_BLOCK_LEN];
-			u8 rx_crypto_buf[6 * AES_BLOCK_LEN];
 		} ccmp;
 		struct {
 			u8 tx_pn[6];
--- a/net/mac80211/cfg.c	2011-07-06 21:40:12.000000000 +0200
+++ b/net/mac80211/cfg.c	2011-07-06 21:59:19.000000000 +0200
@@ -209,6 +209,7 @@ static int ieee80211_get_key(struct wiph
 	u8 seq[6] = {0};
 	struct key_params params;
 	struct ieee80211_key *key = NULL;
+	u64 pn64;
 	u32 iv32;
 	u16 iv16;
 	int err = -ENOENT;
@@ -256,12 +257,13 @@ static int ieee80211_get_key(struct wiph
 		params.seq_len = 6;
 		break;
 	case WLAN_CIPHER_SUITE_CCMP:
-		seq[0] = key->u.ccmp.tx_pn[5];
-		seq[1] = key->u.ccmp.tx_pn[4];
-		seq[2] = key->u.ccmp.tx_pn[3];
-		seq[3] = key->u.ccmp.tx_pn[2];
-		seq[4] = key->u.ccmp.tx_pn[1];
-		seq[5] = key->u.ccmp.tx_pn[0];
+		pn64 = atomic64_read(&key->u.ccmp.tx_pn);
+		seq[0] = pn64;
+		seq[1] = pn64 >> 8;
+		seq[2] = pn64 >> 16;
+		seq[3] = pn64 >> 24;
+		seq[4] = pn64 >> 32;
+		seq[5] = pn64 >> 40;
 		params.seq = seq;
 		params.seq_len = 6;
 		break;
--- a/net/mac80211/wpa.c	2011-07-06 21:40:12.000000000 +0200
+++ b/net/mac80211/wpa.c	2011-07-06 21:59:19.000000000 +0200
@@ -15,6 +15,7 @@
 #include <linux/gfp.h>
 #include <asm/unaligned.h>
 #include <net/mac80211.h>
+#include <crypto/aes.h>
 
 #include "ieee80211_i.h"
 #include "michael.h"
@@ -290,6 +291,8 @@ static void ccmp_special_blocks(struct s
 	unsigned int hdrlen;
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
 
+	memset(scratch, 0, 6 * AES_BLOCK_LEN);
+
 	b_0 = scratch + 3 * AES_BLOCK_LEN;
 	aad = scratch + 4 * AES_BLOCK_LEN;
 
@@ -380,8 +383,10 @@ static int ccmp_encrypt_skb(struct ieee8
 	struct ieee80211_key *key = tx->key;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	int hdrlen, len, tail;
-	u8 *pos, *pn;
-	int i;
+	u8 *pos;
+	u8 pn[6];
+	u64 pn64;
+	u8 scratch[6 * AES_BLOCK_LEN];
 
 	if (info->control.hw_key &&
 	    !(info->control.hw_key->flags & IEEE80211_KEY_FLAG_GENERATE_IV)) {
@@ -409,14 +414,14 @@ static int ccmp_encrypt_skb(struct ieee8
 	hdr = (struct ieee80211_hdr *) pos;
 	pos += hdrlen;
 
-	/* PN = PN + 1 */
-	pn = key->u.ccmp.tx_pn;
+	pn64 = atomic64_inc_return(&key->u.ccmp.tx_pn);
 
-	for (i = CCMP_PN_LEN - 1; i >= 0; i--) {
-		pn[i]++;
-		if (pn[i])
-			break;
-	}
+	pn[5] = pn64;
+	pn[4] = pn64 >> 8;
+	pn[3] = pn64 >> 16;
+	pn[2] = pn64 >> 24;
+	pn[1] = pn64 >> 32;
+	pn[0] = pn64 >> 40;
 
 	ccmp_pn2hdr(pos, pn, key->conf.keyidx);
 
@@ -425,8 +430,8 @@ static int ccmp_encrypt_skb(struct ieee8
 		return 0;
 
 	pos += CCMP_HDR_LEN;
-	ccmp_special_blocks(skb, pn, key->u.ccmp.tx_crypto_buf, 0);
-	ieee80211_aes_ccm_encrypt(key->u.ccmp.tfm, key->u.ccmp.tx_crypto_buf, pos, len,
+	ccmp_special_blocks(skb, pn, scratch, 0);
+	ieee80211_aes_ccm_encrypt(key->u.ccmp.tfm, scratch, pos, len,
 				  pos, skb_put(skb, CCMP_MIC_LEN));
 
 	return 0;
@@ -482,11 +487,12 @@ ieee80211_crypto_ccmp_decrypt(struct iee
 	}
 
 	if (!(status->flag & RX_FLAG_DECRYPTED)) {
+		u8 scratch[6 * AES_BLOCK_LEN];
 		/* hardware didn't decrypt/verify MIC */
-		ccmp_special_blocks(skb, pn, key->u.ccmp.rx_crypto_buf, 1);
+		ccmp_special_blocks(skb, pn, scratch, 1);
 
 		if (ieee80211_aes_ccm_decrypt(
-			    key->u.ccmp.tfm, key->u.ccmp.rx_crypto_buf,
+			    key->u.ccmp.tfm, scratch,
 			    skb->data + hdrlen + CCMP_HDR_LEN, data_len,
 			    skb->data + skb->len - CCMP_MIC_LEN,
 			    skb->data + hdrlen + CCMP_HDR_LEN))
--- a/net/mac80211/debugfs_key.c	2011-07-06 21:39:54.000000000 +0200
+++ b/net/mac80211/debugfs_key.c	2011-07-06 21:59:19.000000000 +0200
@@ -79,6 +79,7 @@ static ssize_t key_tx_spec_read(struct f
 				size_t count, loff_t *ppos)
 {
 	const u8 *tpn;
+	u64 pn;
 	char buf[20];
 	int len;
 	struct ieee80211_key *key = file->private_data;
@@ -94,9 +95,10 @@ static ssize_t key_tx_spec_read(struct f
 				key->u.tkip.tx.iv16);
 		break;
 	case WLAN_CIPHER_SUITE_CCMP:
-		tpn = key->u.ccmp.tx_pn;
+		pn = atomic64_read(&key->u.ccmp.tx_pn);
 		len = scnprintf(buf, sizeof(buf), "%02x%02x%02x%02x%02x%02x\n",
-				tpn[0], tpn[1], tpn[2], tpn[3], tpn[4], tpn[5]);
+				(u8)(pn >> 40), (u8)(pn >> 32), (u8)(pn >> 24),
+				(u8)(pn >> 16), (u8)(pn >> 8), (u8)pn);
 		break;
 	case WLAN_CIPHER_SUITE_AES_CMAC:
 		tpn = key->u.aes_cmac.tx_pn;

[PATCH 3/2] mac80211: fix CMAC races

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

Just like TKIP and CCMP, CMAC has the PN race.
It might not actually be possible to hit it now
since there aren't multiple ACs for management
frames, but fix it anyway.

Also move scratch buffers onto the stack.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/aes_cmac.c    |    8 ++++----
 net/mac80211/aes_cmac.h    |    2 +-
 net/mac80211/cfg.c         |   13 +++++++------
 net/mac80211/debugfs_key.c |    7 +++----
 net/mac80211/key.h         |    5 +----
 net/mac80211/wpa.c         |   30 +++++++++++++++++-------------
 6 files changed, 33 insertions(+), 32 deletions(-)

--- a/net/mac80211/key.h	2011-07-06 21:56:09.000000000 +0200
+++ b/net/mac80211/key.h	2011-07-06 21:57:51.000000000 +0200
@@ -97,14 +97,11 @@ struct ieee80211_key {
 #endif
 		} ccmp;
 		struct {
-			u8 tx_pn[6];
+			atomic64_t tx_pn;
 			u8 rx_pn[6];
 			struct crypto_cipher *tfm;
 			u32 replays; /* dot11RSNAStatsCMACReplays */
 			u32 icverrors; /* dot11RSNAStatsCMACICVErrors */
-			/* scratch buffers for virt_to_page() (crypto API) */
-			u8 tx_crypto_buf[2 * AES_BLOCK_LEN];
-			u8 rx_crypto_buf[2 * AES_BLOCK_LEN];
 		} aes_cmac;
 	} u;
 
--- a/net/mac80211/wpa.c	2011-07-06 21:56:26.000000000 +0200
+++ b/net/mac80211/wpa.c	2011-07-06 21:57:51.000000000 +0200
@@ -523,6 +523,16 @@ static void bip_aad(struct sk_buff *skb,
 }
 
 
+static inline void bip_ipn_set64(u8 *d, u64 pn)
+{
+	*d++ = pn;
+	*d++ = pn >> 8;
+	*d++ = pn >> 16;
+	*d++ = pn >> 24;
+	*d++ = pn >> 32;
+	*d = pn >> 40;
+}
+
 static inline void bip_ipn_swap(u8 *d, const u8 *s)
 {
 	*d++ = s[5];
@@ -541,8 +551,8 @@ ieee80211_crypto_aes_cmac_encrypt(struct
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	struct ieee80211_key *key = tx->key;
 	struct ieee80211_mmie *mmie;
-	u8 *pn, aad[20];
-	int i;
+	u8 aad[20];
+	u64 pn64;
 
 	if (info->control.hw_key)
 		return 0;
@@ -556,22 +566,17 @@ ieee80211_crypto_aes_cmac_encrypt(struct
 	mmie->key_id = cpu_to_le16(key->conf.keyidx);
 
 	/* PN = PN + 1 */
-	pn = key->u.aes_cmac.tx_pn;
+	pn64 = atomic64_inc_return(&key->u.aes_cmac.tx_pn);
 
-	for (i = sizeof(key->u.aes_cmac.tx_pn) - 1; i >= 0; i--) {
-		pn[i]++;
-		if (pn[i])
-			break;
-	}
-	bip_ipn_swap(mmie->sequence_number, pn);
+	bip_ipn_set64(mmie->sequence_number, pn64);
 
 	bip_aad(skb, aad);
 
 	/*
 	 * MIC = AES-128-CMAC(IGTK, AAD || Management Frame Body || MMIE, 64)
 	 */
-	ieee80211_aes_cmac(key->u.aes_cmac.tfm, key->u.aes_cmac.tx_crypto_buf,
-			   aad, skb->data + 24, skb->len - 24, mmie->mic);
+	ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
+			   skb->data + 24, skb->len - 24, mmie->mic);
 
 	return TX_CONTINUE;
 }
@@ -609,8 +614,7 @@ ieee80211_crypto_aes_cmac_decrypt(struct
 	if (!(status->flag & RX_FLAG_DECRYPTED)) {
 		/* hardware didn't decrypt/verify MIC */
 		bip_aad(skb, aad);
-		ieee80211_aes_cmac(key->u.aes_cmac.tfm,
-				   key->u.aes_cmac.rx_crypto_buf, aad,
+		ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
 				   skb->data + 24, skb->len - 24, mic);
 		if (memcmp(mic, mmie->mic, sizeof(mmie->mic)) != 0) {
 			key->u.aes_cmac.icverrors++;
--- a/net/mac80211/aes_cmac.h	2011-07-06 21:56:09.000000000 +0200
+++ b/net/mac80211/aes_cmac.h	2011-07-06 21:57:51.000000000 +0200
@@ -12,7 +12,7 @@
 #include <linux/crypto.h>
 
 struct crypto_cipher * ieee80211_aes_cmac_key_setup(const u8 key[]);
-void ieee80211_aes_cmac(struct crypto_cipher *tfm, u8 *scratch, const u8 *aad,
+void ieee80211_aes_cmac(struct crypto_cipher *tfm, const u8 *aad,
 			const u8 *data, size_t data_len, u8 *mic);
 void ieee80211_aes_cmac_key_free(struct crypto_cipher *tfm);
 
--- a/net/mac80211/aes_cmac.c	2011-07-06 21:56:09.000000000 +0200
+++ b/net/mac80211/aes_cmac.c	2011-07-06 21:57:51.000000000 +0200
@@ -35,10 +35,10 @@ static void gf_mulx(u8 *pad)
 }
 
 
-static void aes_128_cmac_vector(struct crypto_cipher *tfm, u8 *scratch,
-				size_t num_elem,
+static void aes_128_cmac_vector(struct crypto_cipher *tfm, size_t num_elem,
 				const u8 *addr[], const size_t *len, u8 *mac)
 {
+	u8 scratch[2 * AES_BLOCK_SIZE];
 	u8 *cbc, *pad;
 	const u8 *pos, *end;
 	size_t i, e, left, total_len;
@@ -95,7 +95,7 @@ static void aes_128_cmac_vector(struct c
 }
 
 
-void ieee80211_aes_cmac(struct crypto_cipher *tfm, u8 *scratch, const u8 *aad,
+void ieee80211_aes_cmac(struct crypto_cipher *tfm, const u8 *aad,
 			const u8 *data, size_t data_len, u8 *mic)
 {
 	const u8 *addr[3];
@@ -110,7 +110,7 @@ void ieee80211_aes_cmac(struct crypto_ci
 	addr[2] = zero;
 	len[2] = CMAC_TLEN;
 
-	aes_128_cmac_vector(tfm, scratch, 3, addr, len, mic);
+	aes_128_cmac_vector(tfm, 3, addr, len, mic);
 }
 
 
--- a/net/mac80211/cfg.c	2011-07-06 21:56:09.000000000 +0200
+++ b/net/mac80211/cfg.c	2011-07-06 21:57:51.000000000 +0200
@@ -268,12 +268,13 @@ static int ieee80211_get_key(struct wiph
 		params.seq_len = 6;
 		break;
 	case WLAN_CIPHER_SUITE_AES_CMAC:
-		seq[0] = key->u.aes_cmac.tx_pn[5];
-		seq[1] = key->u.aes_cmac.tx_pn[4];
-		seq[2] = key->u.aes_cmac.tx_pn[3];
-		seq[3] = key->u.aes_cmac.tx_pn[2];
-		seq[4] = key->u.aes_cmac.tx_pn[1];
-		seq[5] = key->u.aes_cmac.tx_pn[0];
+		pn64 = atomic64_read(&key->u.aes_cmac.tx_pn);
+		seq[0] = pn64;
+		seq[1] = pn64 >> 8;
+		seq[2] = pn64 >> 16;
+		seq[3] = pn64 >> 24;
+		seq[4] = pn64 >> 32;
+		seq[5] = pn64 >> 40;
 		params.seq = seq;
 		params.seq_len = 6;
 		break;
--- a/net/mac80211/debugfs_key.c	2011-07-06 21:56:09.000000000 +0200
+++ b/net/mac80211/debugfs_key.c	2011-07-06 21:57:51.000000000 +0200
@@ -78,7 +78,6 @@ KEY_OPS(algorithm);
 static ssize_t key_tx_spec_read(struct file *file, char __user *userbuf,
 				size_t count, loff_t *ppos)
 {
-	const u8 *tpn;
 	u64 pn;
 	char buf[20];
 	int len;
@@ -101,10 +100,10 @@ static ssize_t key_tx_spec_read(struct f
 				(u8)(pn >> 16), (u8)(pn >> 8), (u8)pn);
 		break;
 	case WLAN_CIPHER_SUITE_AES_CMAC:
-		tpn = key->u.aes_cmac.tx_pn;
+		pn = atomic64_read(&key->u.aes_cmac.tx_pn);
 		len = scnprintf(buf, sizeof(buf), "%02x%02x%02x%02x%02x%02x\n",
-				tpn[0], tpn[1], tpn[2], tpn[3], tpn[4],
-				tpn[5]);
+				(u8)(pn >> 40), (u8)(pn >> 32), (u8)(pn >> 24),
+				(u8)(pn >> 16), (u8)(pn >> 8), (u8)pn);
 		break;
 	default:
 		return 0;