[PATCH v3 0/5] wifi: mac80211: refactor CMAC implementation

[PATCH v3 0/5] wifi: mac80211: refactor CMAC implementation

[Chien Wong]

This series refactors the CMAC implementation in mac80211 to:
1. Add proper error handling
2. Remove duplication

Every patch in the patchset has been tested with mac80211_hwsim and the
result looks good.
---
Changes in v3:
- Drop patch 1/6("wifi: mac80211: remove an unnecessary copy").
- Rework patch 2/6("wifi: mac80211: fix CMAC functions not handling errors")
  and patch 5/6("wifi: mac80211: refactor CMAC crypt functions") accordingly.
---
Changes in v2:
- Remove words in commit messages stating that only compile is tested.
- Remove Fixes line in patch 1/6.
---
Chien Wong (5):
  wifi: mac80211: fix CMAC functions not handling errors
  wifi: mac80211: add generic MMIE struct defines
  wifi: mac80211: utilize the newly defined CMAC constants
  wifi: mac80211: refactor CMAC crypt functions
  wifi: mac80211: refactor CMAC packet handlers

 include/linux/ieee80211.h |  14 +++-
 net/mac80211/aes_cmac.c   |  64 ++++++++---------
 net/mac80211/aes_cmac.h   |   6 +-
 net/mac80211/aes_gmac.h   |   2 +-
 net/mac80211/rx.c         |   6 +-
 net/mac80211/tx.c         |   6 +-
 net/mac80211/wpa.c        | 144 +++++++-------------------------------
 net/mac80211/wpa.h        |  10 ++-
 8 files changed, 87 insertions(+), 165 deletions(-)

-- 
2.51.2

[PATCH v3 1/5] wifi: mac80211: fix CMAC functions not handling errors

[Chien Wong]

The called hash functions could fail thus we should check return values.
All references to the changed functions in the tree are adapted.

Fixes: 26717828b75d ("mac80211: aes-cmac: switch to shash CMAC driver")
Signed-off-by: Chien Wong <m@xv97.com>
---
 net/mac80211/aes_cmac.c | 63 +++++++++++++++++++++++++++++------------
 net/mac80211/aes_cmac.h |  8 +++---
 net/mac80211/wpa.c      | 20 +++++++------
 3 files changed, 61 insertions(+), 30 deletions(-)

diff --git a/net/mac80211/aes_cmac.c b/net/mac80211/aes_cmac.c
index 48c04f89de20..adce68ea0981 100644
--- a/net/mac80211/aes_cmac.c
+++ b/net/mac80211/aes_cmac.c
@@ -22,50 +22,77 @@
 
 static const u8 zero[CMAC_TLEN_256];
 
-void ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
-			const u8 *data, size_t data_len, u8 *mic)
+int ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
+		       const u8 *data, size_t data_len, u8 *mic)
 {
+	int err;
 	SHASH_DESC_ON_STACK(desc, tfm);
 	u8 out[AES_BLOCK_SIZE];
 	const __le16 *fc;
 
 	desc->tfm = tfm;
 
-	crypto_shash_init(desc);
-	crypto_shash_update(desc, aad, AAD_LEN);
+	err = crypto_shash_init(desc);
+	if (err)
+		goto out;
+	err = crypto_shash_update(desc, aad, AAD_LEN);
+	if (err)
+		goto out;
 	fc = (const __le16 *)aad;
 	if (ieee80211_is_beacon(*fc)) {
 		/* mask Timestamp field to zero */
-		crypto_shash_update(desc, zero, 8);
-		crypto_shash_update(desc, data + 8, data_len - 8 - CMAC_TLEN);
+		err = crypto_shash_update(desc, zero, 8);
+		if (err)
+			goto out;
+		err = crypto_shash_update(desc, data + 8, data_len - 8 - CMAC_TLEN);
+		if (err)
+			goto out;
 	} else {
-		crypto_shash_update(desc, data, data_len - CMAC_TLEN);
+		err = crypto_shash_update(desc, data, data_len - CMAC_TLEN);
+		if (err)
+			goto out;
 	}
-	crypto_shash_finup(desc, zero, CMAC_TLEN, out);
-
+	err = crypto_shash_finup(desc, zero, CMAC_TLEN, out);
+	if (err)
+		goto out;
 	memcpy(mic, out, CMAC_TLEN);
+out:
+	return err;
 }
 
-void ieee80211_aes_cmac_256(struct crypto_shash *tfm, const u8 *aad,
-			    const u8 *data, size_t data_len, u8 *mic)
+int ieee80211_aes_cmac_256(struct crypto_shash *tfm, const u8 *aad,
+			   const u8 *data, size_t data_len, u8 *mic)
 {
+	int err;
 	SHASH_DESC_ON_STACK(desc, tfm);
 	const __le16 *fc;
 
 	desc->tfm = tfm;
 
-	crypto_shash_init(desc);
-	crypto_shash_update(desc, aad, AAD_LEN);
+	err = crypto_shash_init(desc);
+	if (err)
+		goto out;
+	err = crypto_shash_update(desc, aad, AAD_LEN);
+	if (err)
+		goto out;
 	fc = (const __le16 *)aad;
 	if (ieee80211_is_beacon(*fc)) {
 		/* mask Timestamp field to zero */
-		crypto_shash_update(desc, zero, 8);
-		crypto_shash_update(desc, data + 8,
-				    data_len - 8 - CMAC_TLEN_256);
+		err = crypto_shash_update(desc, zero, 8);
+		if (err)
+			goto out;
+		err = crypto_shash_update(desc, data + 8,
+					  data_len - 8 - CMAC_TLEN_256);
+		if (err)
+			goto out;
 	} else {
-		crypto_shash_update(desc, data, data_len - CMAC_TLEN_256);
+		err = crypto_shash_update(desc, data, data_len - CMAC_TLEN_256);
+		if (err)
+			goto out;
 	}
-	crypto_shash_finup(desc, zero, CMAC_TLEN_256, mic);
+	err = crypto_shash_finup(desc, zero, CMAC_TLEN_256, mic);
+out:
+	return err;
 }
 
 struct crypto_shash *ieee80211_aes_cmac_key_setup(const u8 key[],
diff --git a/net/mac80211/aes_cmac.h b/net/mac80211/aes_cmac.h
index 76817446fb83..f74150542142 100644
--- a/net/mac80211/aes_cmac.h
+++ b/net/mac80211/aes_cmac.h
@@ -11,10 +11,10 @@
 
 struct crypto_shash *ieee80211_aes_cmac_key_setup(const u8 key[],
 						  size_t key_len);
-void ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
-			const u8 *data, size_t data_len, u8 *mic);
-void ieee80211_aes_cmac_256(struct crypto_shash *tfm, const u8 *aad,
-			    const u8 *data, size_t data_len, u8 *mic);
+int ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
+		       const u8 *data, size_t data_len, u8 *mic);
+int ieee80211_aes_cmac_256(struct crypto_shash *tfm, const u8 *aad,
+			   const u8 *data, size_t data_len, u8 *mic);
 void ieee80211_aes_cmac_key_free(struct crypto_shash *tfm);
 
 #endif /* AES_CMAC_H */
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index 40d5d9e48479..bb0fa505cdca 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -869,8 +869,9 @@ ieee80211_crypto_aes_cmac_encrypt(struct ieee80211_tx_data *tx)
 	/*
 	 * MIC = AES-128-CMAC(IGTK, AAD || Management Frame Body || MMIE, 64)
 	 */
-	ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
-			   skb->data + 24, skb->len - 24, mmie->mic);
+	if (ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
+			       skb->data + 24, skb->len - 24, mmie->mic))
+		return TX_DROP;
 
 	return TX_CONTINUE;
 }
@@ -916,8 +917,9 @@ ieee80211_crypto_aes_cmac_256_encrypt(struct ieee80211_tx_data *tx)
 
 	/* MIC = AES-256-CMAC(IGTK, AAD || Management Frame Body || MMIE, 128)
 	 */
-	ieee80211_aes_cmac_256(key->u.aes_cmac.tfm, aad,
-			       skb->data + 24, skb->len - 24, mmie->mic);
+	if (ieee80211_aes_cmac_256(key->u.aes_cmac.tfm, aad,
+				   skb->data + 24, skb->len - 24, mmie->mic))
+		return TX_DROP;
 
 	return TX_CONTINUE;
 }
@@ -956,8 +958,9 @@ ieee80211_crypto_aes_cmac_decrypt(struct ieee80211_rx_data *rx)
 	if (!(status->flag & RX_FLAG_DECRYPTED)) {
 		/* hardware didn't decrypt/verify MIC */
 		bip_aad(skb, aad);
-		ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
-				   skb->data + 24, skb->len - 24, mic);
+		if (ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
+				       skb->data + 24, skb->len - 24, mic))
+			return RX_DROP_U_DECRYPT_FAIL;
 		if (crypto_memneq(mic, mmie->mic, sizeof(mmie->mic))) {
 			key->u.aes_cmac.icverrors++;
 			return RX_DROP_U_MIC_FAIL;
@@ -1006,8 +1009,9 @@ ieee80211_crypto_aes_cmac_256_decrypt(struct ieee80211_rx_data *rx)
 	if (!(status->flag & RX_FLAG_DECRYPTED)) {
 		/* hardware didn't decrypt/verify MIC */
 		bip_aad(skb, aad);
-		ieee80211_aes_cmac_256(key->u.aes_cmac.tfm, aad,
-				       skb->data + 24, skb->len - 24, mic);
+		if (ieee80211_aes_cmac_256(key->u.aes_cmac.tfm, aad,
+					   skb->data + 24, skb->len - 24, mic))
+			return RX_DROP_U_DECRYPT_FAIL;
 		if (crypto_memneq(mic, mmie->mic, sizeof(mmie->mic))) {
 			key->u.aes_cmac.icverrors++;
 			return RX_DROP_U_MIC_FAIL;
-- 
2.51.2

[PATCH v3 2/5] wifi: mac80211: add generic MMIE struct defines

[Chien Wong]

The added struct is needed when writing generic handler for both CMAC-128
and CMAC-256.

Signed-off-by: Chien Wong <m@xv97.com>
---
 include/linux/ieee80211.h | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/include/linux/ieee80211.h b/include/linux/ieee80211.h
index ddff9102f633..9392fc11d4fb 100644
--- a/include/linux/ieee80211.h
+++ b/include/linux/ieee80211.h
@@ -1633,7 +1633,7 @@ struct ieee80211_mgmt {
 #define IEEE80211_MIN_ACTION_SIZE offsetof(struct ieee80211_mgmt, u.action.u)
 
 
-/* Management MIC information element (IEEE 802.11w) */
+/* Management MIC information element (IEEE 802.11w) for CMAC */
 struct ieee80211_mmie {
 	u8 element_id;
 	u8 length;
@@ -1651,6 +1651,15 @@ struct ieee80211_mmie_16 {
 	u8 mic[16];
 } __packed;
 
+/* Management MIC information element (IEEE 802.11w) for all variants */
+struct ieee80211_mmie_var {
+	u8 element_id;
+	u8 length;
+	__le16 key_id;
+	u8 sequence_number[6];
+	u8 mic[]; /* 8 or 16 bytes */
+} __packed;
+
 struct ieee80211_vendor_ie {
 	u8 element_id;
 	u8 len;
@@ -4064,6 +4073,9 @@ enum ieee80211_radio_measurement_actioncode {
 #define IEEE80211_GCMP_HDR_LEN		8
 #define IEEE80211_GCMP_MIC_LEN		16
 #define IEEE80211_GCMP_PN_LEN		6
+#define IEEE80211_CMAC_128_MIC_LEN	8
+#define IEEE80211_CMAC_256_MIC_LEN	16
+#define IEEE80211_GMAC_MIC_LEN		16
 
 #define FILS_NONCE_LEN			16
 #define FILS_MAX_KEK_LEN		64
-- 
2.51.2

[PATCH v3 3/5] wifi: mac80211: utilize the newly defined CMAC constants

[Chien Wong]

Make use of the added constants to reduce duplication.

Signed-off-by: Chien Wong <m@xv97.com>
---
 net/mac80211/aes_cmac.c | 4 ++--
 net/mac80211/aes_gmac.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/aes_cmac.c b/net/mac80211/aes_cmac.c
index adce68ea0981..01fb8b6c5dfb 100644
--- a/net/mac80211/aes_cmac.c
+++ b/net/mac80211/aes_cmac.c
@@ -16,8 +16,8 @@
 #include "key.h"
 #include "aes_cmac.h"
 
-#define CMAC_TLEN 8 /* CMAC TLen = 64 bits (8 octets) */
-#define CMAC_TLEN_256 16 /* CMAC TLen = 128 bits (16 octets) */
+#define CMAC_TLEN IEEE80211_CMAC_128_MIC_LEN
+#define CMAC_TLEN_256 IEEE80211_CMAC_256_MIC_LEN
 #define AAD_LEN 20
 
 static const u8 zero[CMAC_TLEN_256];
diff --git a/net/mac80211/aes_gmac.h b/net/mac80211/aes_gmac.h
index c739356bae2a..09378e52c7a6 100644
--- a/net/mac80211/aes_gmac.h
+++ b/net/mac80211/aes_gmac.h
@@ -9,7 +9,7 @@
 #include <linux/crypto.h>
 
 #define GMAC_AAD_LEN	20
-#define GMAC_MIC_LEN	16
+#define GMAC_MIC_LEN	IEEE80211_GMAC_MIC_LEN
 #define GMAC_NONCE_LEN	12
 
 struct crypto_aead *ieee80211_aes_gmac_key_setup(const u8 key[],
-- 
2.51.2

[PATCH v3 4/5] wifi: mac80211: refactor CMAC crypt functions

[Chien Wong]

ieee80211_aes_cmac() and ieee80211_aes_cmac_256() are almost the same.
Merge them. This removes duplication.

It should be noted that the refactored ieee80211_aes_cmac() permits
8 bytes output for CMAC-256 and 16 bytes output for CMAC-128. In such
cases, it would generate result correctly as expected.

All references to the refactored functions in the tree are adapted.

Signed-off-by: Chien Wong <m@xv97.com>
---
 net/mac80211/aes_cmac.c | 53 ++++++++++-------------------------------
 net/mac80211/aes_cmac.h |  4 +---
 net/mac80211/wpa.c      | 16 ++++++++-----
 3 files changed, 24 insertions(+), 49 deletions(-)

diff --git a/net/mac80211/aes_cmac.c b/net/mac80211/aes_cmac.c
index 01fb8b6c5dfb..588e3c9879b3 100644
--- a/net/mac80211/aes_cmac.c
+++ b/net/mac80211/aes_cmac.c
@@ -16,20 +16,21 @@
 #include "key.h"
 #include "aes_cmac.h"
 
-#define CMAC_TLEN IEEE80211_CMAC_128_MIC_LEN
-#define CMAC_TLEN_256 IEEE80211_CMAC_256_MIC_LEN
 #define AAD_LEN 20
 
-static const u8 zero[CMAC_TLEN_256];
+static const u8 zero[IEEE80211_CMAC_256_MIC_LEN];
 
 int ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
-		       const u8 *data, size_t data_len, u8 *mic)
+		       const u8 *data, size_t data_len, u8 *mic, unsigned int mic_len)
 {
 	int err;
 	SHASH_DESC_ON_STACK(desc, tfm);
-	u8 out[AES_BLOCK_SIZE];
 	const __le16 *fc;
 
+	if (mic_len != IEEE80211_CMAC_128_MIC_LEN &&
+	    mic_len != IEEE80211_CMAC_256_MIC_LEN)
+		return -EINVAL;
+
 	desc->tfm = tfm;
 
 	err = crypto_shash_init(desc);
@@ -44,53 +45,25 @@ int ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
 		err = crypto_shash_update(desc, zero, 8);
 		if (err)
 			goto out;
-		err = crypto_shash_update(desc, data + 8, data_len - 8 - CMAC_TLEN);
+		err = crypto_shash_update(desc, data + 8, data_len - 8 - mic_len);
 		if (err)
 			goto out;
 	} else {
-		err = crypto_shash_update(desc, data, data_len - CMAC_TLEN);
+		err = crypto_shash_update(desc, data, data_len - mic_len);
 		if (err)
 			goto out;
 	}
-	err = crypto_shash_finup(desc, zero, CMAC_TLEN, out);
-	if (err)
-		goto out;
-	memcpy(mic, out, CMAC_TLEN);
-out:
-	return err;
-}
 
-int ieee80211_aes_cmac_256(struct crypto_shash *tfm, const u8 *aad,
-			   const u8 *data, size_t data_len, u8 *mic)
-{
-	int err;
-	SHASH_DESC_ON_STACK(desc, tfm);
-	const __le16 *fc;
+	if (mic_len == IEEE80211_CMAC_128_MIC_LEN) {
+		u8 out[AES_BLOCK_SIZE];
 
-	desc->tfm = tfm;
-
-	err = crypto_shash_init(desc);
-	if (err)
-		goto out;
-	err = crypto_shash_update(desc, aad, AAD_LEN);
-	if (err)
-		goto out;
-	fc = (const __le16 *)aad;
-	if (ieee80211_is_beacon(*fc)) {
-		/* mask Timestamp field to zero */
-		err = crypto_shash_update(desc, zero, 8);
-		if (err)
-			goto out;
-		err = crypto_shash_update(desc, data + 8,
-					  data_len - 8 - CMAC_TLEN_256);
+		err = crypto_shash_finup(desc, zero, mic_len, out);
 		if (err)
 			goto out;
+		memcpy(mic, out, mic_len);
 	} else {
-		err = crypto_shash_update(desc, data, data_len - CMAC_TLEN_256);
-		if (err)
-			goto out;
+		err = crypto_shash_finup(desc, zero, mic_len, mic);
 	}
-	err = crypto_shash_finup(desc, zero, CMAC_TLEN_256, mic);
 out:
 	return err;
 }
diff --git a/net/mac80211/aes_cmac.h b/net/mac80211/aes_cmac.h
index f74150542142..631fc3033576 100644
--- a/net/mac80211/aes_cmac.h
+++ b/net/mac80211/aes_cmac.h
@@ -12,9 +12,7 @@
 struct crypto_shash *ieee80211_aes_cmac_key_setup(const u8 key[],
 						  size_t key_len);
 int ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
-		       const u8 *data, size_t data_len, u8 *mic);
-int ieee80211_aes_cmac_256(struct crypto_shash *tfm, const u8 *aad,
-			   const u8 *data, size_t data_len, u8 *mic);
+		       const u8 *data, size_t data_len, u8 *mic, unsigned int mic_len);
 void ieee80211_aes_cmac_key_free(struct crypto_shash *tfm);
 
 #endif /* AES_CMAC_H */
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index bb0fa505cdca..2c1ee4b8e205 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -870,7 +870,8 @@ ieee80211_crypto_aes_cmac_encrypt(struct ieee80211_tx_data *tx)
 	 * MIC = AES-128-CMAC(IGTK, AAD || Management Frame Body || MMIE, 64)
 	 */
 	if (ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
-			       skb->data + 24, skb->len - 24, mmie->mic))
+			       skb->data + 24, skb->len - 24, mmie->mic,
+			       IEEE80211_CMAC_128_MIC_LEN))
 		return TX_DROP;
 
 	return TX_CONTINUE;
@@ -917,8 +918,9 @@ ieee80211_crypto_aes_cmac_256_encrypt(struct ieee80211_tx_data *tx)
 
 	/* MIC = AES-256-CMAC(IGTK, AAD || Management Frame Body || MMIE, 128)
 	 */
-	if (ieee80211_aes_cmac_256(key->u.aes_cmac.tfm, aad,
-				   skb->data + 24, skb->len - 24, mmie->mic))
+	if (ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
+			       skb->data + 24, skb->len - 24, mmie->mic,
+			       IEEE80211_CMAC_256_MIC_LEN))
 		return TX_DROP;
 
 	return TX_CONTINUE;
@@ -959,7 +961,8 @@ ieee80211_crypto_aes_cmac_decrypt(struct ieee80211_rx_data *rx)
 		/* hardware didn't decrypt/verify MIC */
 		bip_aad(skb, aad);
 		if (ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
-				       skb->data + 24, skb->len - 24, mic))
+				       skb->data + 24, skb->len - 24, mic,
+				       IEEE80211_CMAC_128_MIC_LEN))
 			return RX_DROP_U_DECRYPT_FAIL;
 		if (crypto_memneq(mic, mmie->mic, sizeof(mmie->mic))) {
 			key->u.aes_cmac.icverrors++;
@@ -1009,8 +1012,9 @@ ieee80211_crypto_aes_cmac_256_decrypt(struct ieee80211_rx_data *rx)
 	if (!(status->flag & RX_FLAG_DECRYPTED)) {
 		/* hardware didn't decrypt/verify MIC */
 		bip_aad(skb, aad);
-		if (ieee80211_aes_cmac_256(key->u.aes_cmac.tfm, aad,
-					   skb->data + 24, skb->len - 24, mic))
+		if (ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
+				       skb->data + 24, skb->len - 24, mic,
+				       IEEE80211_CMAC_256_MIC_LEN))
 			return RX_DROP_U_DECRYPT_FAIL;
 		if (crypto_memneq(mic, mmie->mic, sizeof(mmie->mic))) {
 			key->u.aes_cmac.icverrors++;
-- 
2.51.2

[PATCH v3 5/5] wifi: mac80211: refactor CMAC packet handlers

[Chien Wong]

Merge CMAC-128 and CMAC-256 handlers since they are almost the same.
This removes duplication.
All references to the refactored functions in the tree are adapted.

The comment 'MIC = AES-128-CMAC(IGTK, AAD ...' is out-dated since CMAC
is also used with BIGTK, as is the comment for CMAC-256. Simply remove
the comments.

Tested-on: mac80211_hwsim

Signed-off-by: Chien Wong <m@xv97.com>
---
 net/mac80211/rx.c  |   6 +-
 net/mac80211/tx.c  |   6 +-
 net/mac80211/wpa.c | 144 ++++++++-------------------------------------
 net/mac80211/wpa.h |  10 ++--
 4 files changed, 35 insertions(+), 131 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 80067ed1da2f..4c1b649b844a 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -2215,10 +2215,12 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx)
 			rx, IEEE80211_CCMP_256_MIC_LEN);
 		break;
 	case WLAN_CIPHER_SUITE_AES_CMAC:
-		result = ieee80211_crypto_aes_cmac_decrypt(rx);
+		result = ieee80211_crypto_aes_cmac_decrypt(
+			rx, IEEE80211_CMAC_128_MIC_LEN);
 		break;
 	case WLAN_CIPHER_SUITE_BIP_CMAC_256:
-		result = ieee80211_crypto_aes_cmac_256_decrypt(rx);
+		result = ieee80211_crypto_aes_cmac_decrypt(
+			rx, IEEE80211_CMAC_256_MIC_LEN);
 		break;
 	case WLAN_CIPHER_SUITE_BIP_GMAC_128:
 	case WLAN_CIPHER_SUITE_BIP_GMAC_256:
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index e7b141c55f7a..9d8b0a25f73c 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1062,9 +1062,11 @@ ieee80211_tx_h_encrypt(struct ieee80211_tx_data *tx)
 		return ieee80211_crypto_ccmp_encrypt(
 			tx, IEEE80211_CCMP_256_MIC_LEN);
 	case WLAN_CIPHER_SUITE_AES_CMAC:
-		return ieee80211_crypto_aes_cmac_encrypt(tx);
+		return ieee80211_crypto_aes_cmac_encrypt(
+			tx, IEEE80211_CMAC_128_MIC_LEN);
 	case WLAN_CIPHER_SUITE_BIP_CMAC_256:
-		return ieee80211_crypto_aes_cmac_256_encrypt(tx);
+		return ieee80211_crypto_aes_cmac_encrypt(
+			tx, IEEE80211_CMAC_256_MIC_LEN);
 	case WLAN_CIPHER_SUITE_BIP_GMAC_128:
 	case WLAN_CIPHER_SUITE_BIP_GMAC_256:
 		return ieee80211_crypto_aes_gmac_encrypt(tx);
diff --git a/net/mac80211/wpa.c b/net/mac80211/wpa.c
index 2c1ee4b8e205..2f8bd5216542 100644
--- a/net/mac80211/wpa.c
+++ b/net/mac80211/wpa.c
@@ -828,12 +828,14 @@ static inline void bip_ipn_swap(u8 *d, const u8 *s)
 
 
 ieee80211_tx_result
-ieee80211_crypto_aes_cmac_encrypt(struct ieee80211_tx_data *tx)
+ieee80211_crypto_aes_cmac_encrypt(struct ieee80211_tx_data *tx,
+				  unsigned int mic_len)
 {
 	struct sk_buff *skb;
 	struct ieee80211_tx_info *info;
 	struct ieee80211_key *key = tx->key;
-	struct ieee80211_mmie *mmie;
+	struct ieee80211_mmie_var *mmie;
+	size_t mmie_len;
 	u8 aad[20];
 	u64 pn64;
 
@@ -848,62 +850,14 @@ ieee80211_crypto_aes_cmac_encrypt(struct ieee80211_tx_data *tx)
 	    !(key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_MMIE))
 		return TX_CONTINUE;
 
-	if (WARN_ON(skb_tailroom(skb) < sizeof(*mmie)))
-		return TX_DROP;
-
-	mmie = skb_put(skb, sizeof(*mmie));
-	mmie->element_id = WLAN_EID_MMIE;
-	mmie->length = sizeof(*mmie) - 2;
-	mmie->key_id = cpu_to_le16(key->conf.keyidx);
+	mmie_len = sizeof(*mmie) + mic_len;
 
-	/* PN = PN + 1 */
-	pn64 = atomic64_inc_return(&key->conf.tx_pn);
-
-	bip_ipn_set64(mmie->sequence_number, pn64);
-
-	if (info->control.hw_key)
-		return TX_CONTINUE;
-
-	bip_aad(skb, aad);
-
-	/*
-	 * MIC = AES-128-CMAC(IGTK, AAD || Management Frame Body || MMIE, 64)
-	 */
-	if (ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
-			       skb->data + 24, skb->len - 24, mmie->mic,
-			       IEEE80211_CMAC_128_MIC_LEN))
+	if (WARN_ON(skb_tailroom(skb) < mmie_len))
 		return TX_DROP;
 
-	return TX_CONTINUE;
-}
-
-ieee80211_tx_result
-ieee80211_crypto_aes_cmac_256_encrypt(struct ieee80211_tx_data *tx)
-{
-	struct sk_buff *skb;
-	struct ieee80211_tx_info *info;
-	struct ieee80211_key *key = tx->key;
-	struct ieee80211_mmie_16 *mmie;
-	u8 aad[20];
-	u64 pn64;
-
-	if (WARN_ON(skb_queue_len(&tx->skbs) != 1))
-		return TX_DROP;
-
-	skb = skb_peek(&tx->skbs);
-
-	info = IEEE80211_SKB_CB(skb);
-
-	if (info->control.hw_key &&
-	    !(key->conf.flags & IEEE80211_KEY_FLAG_GENERATE_MMIE))
-		return TX_CONTINUE;
-
-	if (WARN_ON(skb_tailroom(skb) < sizeof(*mmie)))
-		return TX_DROP;
-
-	mmie = skb_put(skb, sizeof(*mmie));
+	mmie = skb_put(skb, mmie_len);
 	mmie->element_id = WLAN_EID_MMIE;
-	mmie->length = sizeof(*mmie) - 2;
+	mmie->length = mmie_len - 2;
 	mmie->key_id = cpu_to_le16(key->conf.keyidx);
 
 	/* PN = PN + 1 */
@@ -916,90 +870,39 @@ ieee80211_crypto_aes_cmac_256_encrypt(struct ieee80211_tx_data *tx)
 
 	bip_aad(skb, aad);
 
-	/* MIC = AES-256-CMAC(IGTK, AAD || Management Frame Body || MMIE, 128)
-	 */
 	if (ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
-			       skb->data + 24, skb->len - 24, mmie->mic,
-			       IEEE80211_CMAC_256_MIC_LEN))
+			       skb->data + 24, skb->len - 24, mmie->mic, mic_len))
 		return TX_DROP;
 
 	return TX_CONTINUE;
 }
 
 ieee80211_rx_result
-ieee80211_crypto_aes_cmac_decrypt(struct ieee80211_rx_data *rx)
+ieee80211_crypto_aes_cmac_decrypt(struct ieee80211_rx_data *rx,
+				  unsigned int mic_len)
 {
 	struct sk_buff *skb = rx->skb;
 	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
 	struct ieee80211_key *key = rx->key;
-	struct ieee80211_mmie *mmie;
-	u8 aad[20], mic[8], ipn[6];
+	struct ieee80211_mmie_var *mmie;
+	size_t mmie_len;
+	u8 aad[20], mic[IEEE80211_CMAC_256_MIC_LEN], ipn[6];
 	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
 
 	if (!ieee80211_is_mgmt(hdr->frame_control))
 		return RX_CONTINUE;
 
-	/* management frames are already linear */
-
-	if (skb->len < 24 + sizeof(*mmie))
-		return RX_DROP_U_SHORT_CMAC;
-
-	mmie = (struct ieee80211_mmie *)
-		(skb->data + skb->len - sizeof(*mmie));
-	if (mmie->element_id != WLAN_EID_MMIE ||
-	    mmie->length != sizeof(*mmie) - 2)
-		return RX_DROP_U_BAD_MMIE; /* Invalid MMIE */
-
-	bip_ipn_swap(ipn, mmie->sequence_number);
-
-	if (memcmp(ipn, key->u.aes_cmac.rx_pn, 6) <= 0) {
-		key->u.aes_cmac.replays++;
-		return RX_DROP_U_REPLAY;
-	}
-
-	if (!(status->flag & RX_FLAG_DECRYPTED)) {
-		/* hardware didn't decrypt/verify MIC */
-		bip_aad(skb, aad);
-		if (ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
-				       skb->data + 24, skb->len - 24, mic,
-				       IEEE80211_CMAC_128_MIC_LEN))
-			return RX_DROP_U_DECRYPT_FAIL;
-		if (crypto_memneq(mic, mmie->mic, sizeof(mmie->mic))) {
-			key->u.aes_cmac.icverrors++;
-			return RX_DROP_U_MIC_FAIL;
-		}
-	}
-
-	memcpy(key->u.aes_cmac.rx_pn, ipn, 6);
-
-	/* Remove MMIE */
-	skb_trim(skb, skb->len - sizeof(*mmie));
-
-	return RX_CONTINUE;
-}
-
-ieee80211_rx_result
-ieee80211_crypto_aes_cmac_256_decrypt(struct ieee80211_rx_data *rx)
-{
-	struct sk_buff *skb = rx->skb;
-	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
-	struct ieee80211_key *key = rx->key;
-	struct ieee80211_mmie_16 *mmie;
-	u8 aad[20], mic[16], ipn[6];
-	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
-
-	if (!ieee80211_is_mgmt(hdr->frame_control))
-		return RX_CONTINUE;
+	mmie_len = sizeof(*mmie) + mic_len;
 
 	/* management frames are already linear */
 
-	if (skb->len < 24 + sizeof(*mmie))
-		return RX_DROP_U_SHORT_CMAC256;
+	if (skb->len < 24 + mmie_len)
+		return mic_len == IEEE80211_CMAC_128_MIC_LEN ?
+				  RX_DROP_U_SHORT_CMAC : RX_DROP_U_SHORT_CMAC256;
 
-	mmie = (struct ieee80211_mmie_16 *)
-		(skb->data + skb->len - sizeof(*mmie));
+	mmie = (struct ieee80211_mmie_var *)(skb->data + skb->len - mmie_len);
 	if (mmie->element_id != WLAN_EID_MMIE ||
-	    mmie->length != sizeof(*mmie) - 2)
+	    mmie->length != mmie_len - 2)
 		return RX_DROP_U_BAD_MMIE; /* Invalid MMIE */
 
 	bip_ipn_swap(ipn, mmie->sequence_number);
@@ -1013,10 +916,9 @@ ieee80211_crypto_aes_cmac_256_decrypt(struct ieee80211_rx_data *rx)
 		/* hardware didn't decrypt/verify MIC */
 		bip_aad(skb, aad);
 		if (ieee80211_aes_cmac(key->u.aes_cmac.tfm, aad,
-				       skb->data + 24, skb->len - 24, mic,
-				       IEEE80211_CMAC_256_MIC_LEN))
+				       skb->data + 24, skb->len - 24, mic, mic_len))
 			return RX_DROP_U_DECRYPT_FAIL;
-		if (crypto_memneq(mic, mmie->mic, sizeof(mmie->mic))) {
+		if (crypto_memneq(mic, mmie->mic, mic_len)) {
 			key->u.aes_cmac.icverrors++;
 			return RX_DROP_U_MIC_FAIL;
 		}
@@ -1025,7 +927,7 @@ ieee80211_crypto_aes_cmac_256_decrypt(struct ieee80211_rx_data *rx)
 	memcpy(key->u.aes_cmac.rx_pn, ipn, 6);
 
 	/* Remove MMIE */
-	skb_trim(skb, skb->len - sizeof(*mmie));
+	skb_trim(skb, skb->len - mmie_len);
 
 	return RX_CONTINUE;
 }
diff --git a/net/mac80211/wpa.h b/net/mac80211/wpa.h
index a9a81abb5479..6e8846dfe710 100644
--- a/net/mac80211/wpa.h
+++ b/net/mac80211/wpa.h
@@ -29,13 +29,11 @@ ieee80211_crypto_ccmp_decrypt(struct ieee80211_rx_data *rx,
 			      unsigned int mic_len);
 
 ieee80211_tx_result
-ieee80211_crypto_aes_cmac_encrypt(struct ieee80211_tx_data *tx);
-ieee80211_tx_result
-ieee80211_crypto_aes_cmac_256_encrypt(struct ieee80211_tx_data *tx);
-ieee80211_rx_result
-ieee80211_crypto_aes_cmac_decrypt(struct ieee80211_rx_data *rx);
+ieee80211_crypto_aes_cmac_encrypt(struct ieee80211_tx_data *tx,
+				  unsigned int mic_len);
 ieee80211_rx_result
-ieee80211_crypto_aes_cmac_256_decrypt(struct ieee80211_rx_data *rx);
+ieee80211_crypto_aes_cmac_decrypt(struct ieee80211_rx_data *rx,
+				  unsigned int mic_len);
 ieee80211_tx_result
 ieee80211_crypto_aes_gmac_encrypt(struct ieee80211_tx_data *tx);
 ieee80211_rx_result
-- 
2.51.2

Re: [PATCH v3 1/5] wifi: mac80211: fix CMAC functions not handling errors

[Johannes Berg]

On Tue, 2025-11-11 at 22:57 +0800, Chien Wong wrote:
> 
> -void ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
> -			const u8 *data, size_t data_len, u8 *mic)
> +int ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
> +		       const u8 *data, size_t data_len, u8 *mic)
>  {
> +	int err;
>  	SHASH_DESC_ON_STACK(desc, tfm);
>  	u8 out[AES_BLOCK_SIZE];
>  	const __le16 *fc;
>  
>  	desc->tfm = tfm;
>  
> -	crypto_shash_init(desc);
> -	crypto_shash_update(desc, aad, AAD_LEN);
> +	err = crypto_shash_init(desc);
> +	if (err)
> +		goto out;
> +	err = crypto_shash_update(desc, aad, AAD_LEN);
> +	if (err)
> +		goto out;


Not sure all the 'goto', without

> +out:
> +	return err;

anything other than a return is really better than just returning early?
It's not like this code will change soon again and need more error
handling ;-)

johannes

Re: [PATCH v3 3/5] wifi: mac80211: utilize the newly defined CMAC constants

[Johannes Berg]

On Tue, 2025-11-11 at 22:57 +0800, Chien Wong wrote:
> Make use of the added constants to reduce duplication.
> 
> Signed-off-by: Chien Wong <m@xv97.com>
> ---
>  net/mac80211/aes_cmac.c | 4 ++--
>  net/mac80211/aes_gmac.h | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/net/mac80211/aes_cmac.c b/net/mac80211/aes_cmac.c
> index adce68ea0981..01fb8b6c5dfb 100644
> --- a/net/mac80211/aes_cmac.c
> +++ b/net/mac80211/aes_cmac.c
> @@ -16,8 +16,8 @@
>  #include "key.h"
>  #include "aes_cmac.h"
>  
> -#define CMAC_TLEN 8 /* CMAC TLen = 64 bits (8 octets) */
> -#define CMAC_TLEN_256 16 /* CMAC TLen = 128 bits (16 octets) */
> +#define CMAC_TLEN IEEE80211_CMAC_128_MIC_LEN
> +#define CMAC_TLEN_256 IEEE80211_CMAC_256_MIC_LEN
>  #define AAD_LEN 20
>  
>  static const u8 zero[CMAC_TLEN_256];
> diff --git a/net/mac80211/aes_gmac.h b/net/mac80211/aes_gmac.h
> index c739356bae2a..09378e52c7a6 100644
> --- a/net/mac80211/aes_gmac.h
> +++ b/net/mac80211/aes_gmac.h
> @@ -9,7 +9,7 @@
>  #include <linux/crypto.h>
>  
>  #define GMAC_AAD_LEN	20
> -#define GMAC_MIC_LEN	16
> +#define GMAC_MIC_LEN	IEEE80211_GMAC_MIC_LEN

Might really be better to just switch the code to the new ones?

johannes

Re: [PATCH v3 4/5] wifi: mac80211: refactor CMAC crypt functions

[Johannes Berg]

On Tue, 2025-11-11 at 22:57 +0800, Chien Wong wrote:
> ieee80211_aes_cmac() and ieee80211_aes_cmac_256() are almost the same.
> Merge them. This removes duplication.
> 
> It should be noted that the refactored ieee80211_aes_cmac() permits
> 8 bytes output for CMAC-256 and 16 bytes output for CMAC-128. In such
> cases, it would generate result correctly as expected.

I think you got that the wrong way around? Or I don't understand it.

> All references to the refactored functions in the tree are adapted.

Not sure what that means? You also had that comment in another place? Do
you literally mean "I had to change the callers"? That seems obvious?

> 
> Signed-off-by: Chien Wong <m@xv97.com>
> ---
>  net/mac80211/aes_cmac.c | 53 ++++++++++-------------------------------
>  net/mac80211/aes_cmac.h |  4 +---
>  net/mac80211/wpa.c      | 16 ++++++++-----
>  3 files changed, 24 insertions(+), 49 deletions(-)
> 
> diff --git a/net/mac80211/aes_cmac.c b/net/mac80211/aes_cmac.c
> index 01fb8b6c5dfb..588e3c9879b3 100644
> --- a/net/mac80211/aes_cmac.c
> +++ b/net/mac80211/aes_cmac.c
> @@ -16,20 +16,21 @@
>  #include "key.h"
>  #include "aes_cmac.h"
>  
> -#define CMAC_TLEN IEEE80211_CMAC_128_MIC_LEN
> -#define CMAC_TLEN_256 IEEE80211_CMAC_256_MIC_LEN

Oh, now you did? OK I guess that's fine too.

>  #define AAD_LEN 20
>  
> -static const u8 zero[CMAC_TLEN_256];
> +static const u8 zero[IEEE80211_CMAC_256_MIC_LEN];
>  
>  int ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
> -		       const u8 *data, size_t data_len, u8 *mic)
> +		       const u8 *data, size_t data_len, u8 *mic, unsigned int mic_len)

could trivially keep that in 80 cols, and that would even look better :)

>  {
>  	int err;
>  	SHASH_DESC_ON_STACK(desc, tfm);
> -	u8 out[AES_BLOCK_SIZE];
>  	const __le16 *fc;
>  
> +	if (mic_len != IEEE80211_CMAC_128_MIC_LEN &&
> +	    mic_len != IEEE80211_CMAC_256_MIC_LEN)
> +		return -EINVAL;

I guess it's a bit annoying that we need this, but OTOH this is not the
optimal path (it's SW crypto...) so I guess doesn't matter.

> +	if (mic_len == IEEE80211_CMAC_128_MIC_LEN) {
> +		u8 out[AES_BLOCK_SIZE];
> 
> +		err = crypto_shash_finup(desc, zero, mic_len, out);
>  		if (err)
>  			goto out;
> +		memcpy(mic, out, mic_len);
>  	} else {
> 
> +		err = crypto_shash_finup(desc, zero, mic_len, mic);
>  	}

Arguably, since it's a SW fixup path, you could also just always copy,
using the mic_len == IEEE80211_CMAC_128_MIC_LEN path (since it copies
mic_len).

johannes

Re: [PATCH v3 1/5] wifi: mac80211: fix CMAC functions not handling errors

[Chien Wong]

On 11/12/25 9:11 PM, Johannes Berg wrote:
> On Tue, 2025-11-11 at 22:57 +0800, Chien Wong wrote:
>>
>> -void ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
>> -			const u8 *data, size_t data_len, u8 *mic)
>> +int ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
>> +		       const u8 *data, size_t data_len, u8 *mic)
>>   {
>> +	int err;
>>   	SHASH_DESC_ON_STACK(desc, tfm);
>>   	u8 out[AES_BLOCK_SIZE];
>>   	const __le16 *fc;
>>   
>>   	desc->tfm = tfm;
>>   
>> -	crypto_shash_init(desc);
>> -	crypto_shash_update(desc, aad, AAD_LEN);
>> +	err = crypto_shash_init(desc);
>> +	if (err)
>> +		goto out;
>> +	err = crypto_shash_update(desc, aad, AAD_LEN);
>> +	if (err)
>> +		goto out;
> 
> 
> Not sure all the 'goto', without
> 
>> +out:
>> +	return err;
> 
> anything other than a return is really better than just returning early?
> It's not like this code will change soon again and need more error
> handling ;-)
> 
> johannes

OK. They will be change into simply returning.

Re: [PATCH v3 3/5] wifi: mac80211: utilize the newly defined CMAC constants

[Chien Wong]

On 11/12/25 9:12 PM, Johannes Berg wrote:
> On Tue, 2025-11-11 at 22:57 +0800, Chien Wong wrote:
>> Make use of the added constants to reduce duplication.
>>
>> Signed-off-by: Chien Wong <m@xv97.com>
>> ---
>>   net/mac80211/aes_cmac.c | 4 ++--
>>   net/mac80211/aes_gmac.h | 2 +-
>>   2 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/net/mac80211/aes_cmac.c b/net/mac80211/aes_cmac.c
>> index adce68ea0981..01fb8b6c5dfb 100644
>> --- a/net/mac80211/aes_cmac.c
>> +++ b/net/mac80211/aes_cmac.c
>> @@ -16,8 +16,8 @@
>>   #include "key.h"
>>   #include "aes_cmac.h"
>>   
>> -#define CMAC_TLEN 8 /* CMAC TLen = 64 bits (8 octets) */
>> -#define CMAC_TLEN_256 16 /* CMAC TLen = 128 bits (16 octets) */
>> +#define CMAC_TLEN IEEE80211_CMAC_128_MIC_LEN
>> +#define CMAC_TLEN_256 IEEE80211_CMAC_256_MIC_LEN
>>   #define AAD_LEN 20
>>   
>>   static const u8 zero[CMAC_TLEN_256];
>> diff --git a/net/mac80211/aes_gmac.h b/net/mac80211/aes_gmac.h
>> index c739356bae2a..09378e52c7a6 100644
>> --- a/net/mac80211/aes_gmac.h
>> +++ b/net/mac80211/aes_gmac.h
>> @@ -9,7 +9,7 @@
>>   #include <linux/crypto.h>
>>   
>>   #define GMAC_AAD_LEN	20
>> -#define GMAC_MIC_LEN	16
>> +#define GMAC_MIC_LEN	IEEE80211_GMAC_MIC_LEN
> 
> Might really be better to just switch the code to the new ones?
> 
> johannes
They will be replaced by direct usage in code.

Re: [PATCH v3 4/5] wifi: mac80211: refactor CMAC crypt functions

[Chien Wong]

On 11/12/25 9:19 PM, Johannes Berg wrote:
> On Tue, 2025-11-11 at 22:57 +0800, Chien Wong wrote:
>> ieee80211_aes_cmac() and ieee80211_aes_cmac_256() are almost the same.
>> Merge them. This removes duplication.
>>
>> It should be noted that the refactored ieee80211_aes_cmac() permits
>> 8 bytes output for CMAC-256 and 16 bytes output for CMAC-128. In such
>> cases, it would generate result correctly as expected.
> 
> I think you got that the wrong way around? Or I don't understand it.

I mean that the functions do allow caller using in uncommon ways and the 
behavior is not changed after refactor. Anyway, this is obvious from the 
code. I'd remove the words.>> All references to the refactored functions 
in the tree are adapted.
> 
> Not sure what that means? You also had that comment in another place? Do
> you literally mean "I had to change the callers"? That seems obvious?

Yes, I mean I have changed the callers. This is obvious and the words 
should be removed.
>>   int ieee80211_aes_cmac(struct crypto_shash *tfm, const u8 *aad,
>> -		       const u8 *data, size_t data_len, u8 *mic)
>> +		       const u8 *data, size_t data_len, u8 *mic, unsigned int mic_len)
> 
> could trivially keep that in 80 cols, and that would even look better :)
> 
Sure.>>   {
>>   	int err;
>>   	SHASH_DESC_ON_STACK(desc, tfm);
>> -	u8 out[AES_BLOCK_SIZE];
>>   	const __le16 *fc;
>>   
>> +	if (mic_len != IEEE80211_CMAC_128_MIC_LEN &&
>> +	    mic_len != IEEE80211_CMAC_256_MIC_LEN)
>> +		return -EINVAL;
> 
> I guess it's a bit annoying that we need this, but OTOH this is not the
> optimal path (it's SW crypto...) so I guess doesn't matter.
> 

I feel the same. Validating parameters is unnecessary here.

>> +	if (mic_len == IEEE80211_CMAC_128_MIC_LEN) {
>> +		u8 out[AES_BLOCK_SIZE];
>>
>> +		err = crypto_shash_finup(desc, zero, mic_len, out);
>>   		if (err)
>>   			goto out;
>> +		memcpy(mic, out, mic_len);
>>   	} else {
>>
>> +		err = crypto_shash_finup(desc, zero, mic_len, mic);
>>   	}
> 
> Arguably, since it's a SW fixup path, you could also just always copy,
> using the mic_len == IEEE80211_CMAC_128_MIC_LEN path (since it copies
> mic_len).

OK. The little optimization here is unnecessary.