The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH 0/6] crypto: Add __cleanup functions for zeroizing aes_cmac_key & aes_cmac_ctx
@ 2026-08-05 14:36 Thomas Huth
  2026-08-05 14:36 ` [PATCH 1/6] crypto: Provide wrapper functions for zeroizing aes_cmac_key and aes_cmac_ctx Thomas Huth
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Thomas Huth @ 2026-08-05 14:36 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Eric Biggers; +Cc: linux-crypto, linux-kernel

Code that uses AES-CMAC might need to zeroize their local aes_cmac_key
and/or aes_cmac_ctx structures after use to avoid leaking sensitive
material on the stack.

The first patch introduces an aes_cmac_zeroize_key() and an
aes_cmac_zeroize_ctx() helper function that can be used with __cleanup()
to automatically clear the key and context when they go out of scope.

Patches 2 - 3 add the __cleanup markers to spots in the code that
missed to clean up the structures so far.

The final two patches are just cosmetics and change some memzero_explicit()
calls to use the __cleanup() way instead.

Thomas Huth (6):
  crypto: Provide wrapper functions for zeroizing aes_cmac_key and
    aes_cmac_ctx
  smb: clear the aes_cmac_key and aes_cmac_ctx when done
  net/tcp-ao: clear the aes_cmac_key when done
  Bluetooth: SMP: clear the aes_cmac_key when done
  lib/crypto: aes: Use _cleanup() for aes_cmac_key instead of
    memzero_explicit()
  mac80211: fils_aead: Use _cleanup for aes_cmac_key instead of
    memzero_explicit

 fs/smb/client/smb2transport.c |  4 ++--
 fs/smb/server/auth.c          |  2 +-
 include/crypto/aes-cbc-macs.h | 26 ++++++++++++++++++++++++++
 lib/crypto/aes.c              |  3 +--
 net/bluetooth/smp.c           |  2 +-
 net/ipv4/tcp_ao.c             |  2 +-
 net/mac80211/fils_aead.c      |  3 +--
 7 files changed, 33 insertions(+), 9 deletions(-)

-- 
2.55.0


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/6] crypto: Provide wrapper functions for zeroizing aes_cmac_key and aes_cmac_ctx
  2026-08-05 14:36 [PATCH 0/6] crypto: Add __cleanup functions for zeroizing aes_cmac_key & aes_cmac_ctx Thomas Huth
@ 2026-08-05 14:36 ` Thomas Huth
  2026-08-05 20:37   ` Eric Biggers
  2026-08-05 20:46   ` Eric Biggers
  2026-08-05 14:36 ` [PATCH 2/6] smb: clear the aes_cmac_key and aes_cmac_ctx when done Thomas Huth
                   ` (4 subsequent siblings)
  5 siblings, 2 replies; 14+ messages in thread
From: Thomas Huth @ 2026-08-05 14:36 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Eric Biggers; +Cc: linux-crypto, linux-kernel

From: Thomas Huth <thuth@redhat.com>

Code that uses AES-CMAC might need to zeroize their local aes_cmac_key
and/or aes_cmac_ctx structures after use to avoid leaking sensitive
material on the stack.

Provide an aes_cmac_zeroize_key() and an aes_cmac_zeroize_ctx() helper
function that can be used with __cleanup() to automatically clear
the key and context when they go out of scope.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/crypto/aes-cbc-macs.h | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/include/crypto/aes-cbc-macs.h b/include/crypto/aes-cbc-macs.h
index e61df108b926d..b2f3b8be7886b 100644
--- a/include/crypto/aes-cbc-macs.h
+++ b/include/crypto/aes-cbc-macs.h
@@ -109,6 +109,32 @@ void aes_cmac_update(struct aes_cmac_ctx *ctx, const u8 *data, size_t data_len);
  */
 void aes_cmac_final(struct aes_cmac_ctx *ctx, u8 out[at_least AES_BLOCK_SIZE]);
 
+/**
+ * aes_cmac_zeroize_key - Clear a aes_cmac_key structure
+ * @ctx: The location of the key structure that should be zeroized
+ *
+ * Explicitly fills the aes_cmac_key with zeroes. This should be done once
+ * the key is not required anymore to avoid that its contents are leaked
+ * on the stack or heap.
+ */
+static inline void aes_cmac_zeroize_key(struct aes_cmac_key *key)
+{
+	memzero_explicit(key, sizeof(*key));
+}
+
+/**
+ * aes_cmac_zeroize_ctx - Clear a aes_cmac_ctx structure
+ * @ctx: The location of the context that should be zeroized
+ *
+ * Explicitly fills the aes_cmac_ctx with zeroes. This should be done once
+ * the context is not required anymore to avoid that its contents are
+ * leaked on the stack or heap. Only required if not using aes_cmac_final().
+ */
+static inline void aes_cmac_zeroize_ctx(struct aes_cmac_ctx *ctx)
+{
+	memzero_explicit(ctx, sizeof(*ctx));
+}
+
 /**
  * aes_cmac() - Compute AES-CMAC or AES-XCBC-MAC in one shot
  * @key: The key to use
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/6] smb: clear the aes_cmac_key and aes_cmac_ctx when done
  2026-08-05 14:36 [PATCH 0/6] crypto: Add __cleanup functions for zeroizing aes_cmac_key & aes_cmac_ctx Thomas Huth
  2026-08-05 14:36 ` [PATCH 1/6] crypto: Provide wrapper functions for zeroizing aes_cmac_key and aes_cmac_ctx Thomas Huth
@ 2026-08-05 14:36 ` Thomas Huth
  2026-08-05 21:12   ` Eric Biggers
  2026-08-05 14:36 ` [PATCH 3/6] net/tcp-ao: clear the aes_cmac_key " Thomas Huth
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Thomas Huth @ 2026-08-05 14:36 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Eric Biggers, Steve French,
	Namjae Jeon
  Cc: linux-crypto, linux-kernel, Paulo Alcantara, Ronnie Sahlberg,
	Shyam Prasad N, Tom Talpey, Bharath SM, Sergey Senozhatsky,
	linux-cifs, samba-technical

From: Thomas Huth <thuth@redhat.com>

Clear the local crypto-related structures via __cleanup() functions
when we're done with them to avoid that sensitive data could leak on
the stack.

Note: cmac_ctx in ksmbd_sign_smb3_pdu() gets cleared in aes_cmac_final()
already, so this does not need a __cleanup() marker.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 fs/smb/client/smb2transport.c | 4 ++--
 fs/smb/server/auth.c          | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/fs/smb/client/smb2transport.c b/fs/smb/client/smb2transport.c
index 1143ee52470a7..d23566da2ac81 100644
--- a/fs/smb/client/smb2transport.c
+++ b/fs/smb/client/smb2transport.c
@@ -464,8 +464,8 @@ smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server)
 	unsigned char smb3_signature[SMB2_CMACAES_SIZE];
 	struct kvec *iov = rqst->rq_iov;
 	struct smb2_hdr *shdr = (struct smb2_hdr *)iov[0].iov_base;
-	struct aes_cmac_key cmac_key;
-	struct aes_cmac_ctx cmac_ctx;
+	struct aes_cmac_key cmac_key __cleanup(aes_cmac_zeroize_key);
+	struct aes_cmac_ctx cmac_ctx __cleanup(aes_cmac_zeroize_ctx);
 	struct smb_rqst drqst;
 	u8 key[SMB3_SIGN_KEY_SIZE];
 
diff --git a/fs/smb/server/auth.c b/fs/smb/server/auth.c
index 86f521e849d5e..f123e6cf9c424 100644
--- a/fs/smb/server/auth.c
+++ b/fs/smb/server/auth.c
@@ -505,7 +505,7 @@ void ksmbd_sign_smb2_pdu(struct ksmbd_conn *conn, char *key, struct kvec *iov,
 void ksmbd_sign_smb3_pdu(struct ksmbd_conn *conn, char *key, struct kvec *iov,
 			 int n_vec, char *sig)
 {
-	struct aes_cmac_key cmac_key;
+	struct aes_cmac_key cmac_key __cleanup(aes_cmac_zeroize_key);
 	struct aes_cmac_ctx cmac_ctx;
 	int i;
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/6] net/tcp-ao: clear the aes_cmac_key when done
  2026-08-05 14:36 [PATCH 0/6] crypto: Add __cleanup functions for zeroizing aes_cmac_key & aes_cmac_ctx Thomas Huth
  2026-08-05 14:36 ` [PATCH 1/6] crypto: Provide wrapper functions for zeroizing aes_cmac_key and aes_cmac_ctx Thomas Huth
  2026-08-05 14:36 ` [PATCH 2/6] smb: clear the aes_cmac_key and aes_cmac_ctx when done Thomas Huth
@ 2026-08-05 14:36 ` Thomas Huth
  2026-08-05 21:02   ` Eric Biggers
  2026-08-05 14:36 ` [PATCH 4/6] Bluetooth: SMP: " Thomas Huth
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Thomas Huth @ 2026-08-05 14:36 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Eric Biggers, Eric Dumazet,
	Neal Cardwell, Jakub Kicinski, Paolo Abeni
  Cc: linux-crypto, linux-kernel, Kuniyuki Iwashima, Simon Horman,
	netdev

From: Thomas Huth <thuth@redhat.com>

Clear the local aes_cmac_key structure via __cleanup() function
when we're done with it to avoid that sensitive data could leak on
the stack.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 net/ipv4/tcp_ao.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
index a56bb79e15e0e..12c724fed8a26 100644
--- a/net/ipv4/tcp_ao.c
+++ b/net/ipv4/tcp_ao.c
@@ -141,7 +141,7 @@ void tcp_ao_calc_traffic_key(const struct tcp_ao_key *mkt, u8 *traffic_key,
 					traffic_key);
 		return;
 	case TCP_AO_ALGO_AES_128_CMAC: {
-		struct aes_cmac_key k;
+		struct aes_cmac_key k __cleanup(aes_cmac_zeroize_key);
 
 		aes_cmac_preparekey(&k, mkt->key, AES_KEYSIZE_128);
 		aes_cmac(&k, input, input_len, traffic_key);
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/6] Bluetooth: SMP: clear the aes_cmac_key when done
  2026-08-05 14:36 [PATCH 0/6] crypto: Add __cleanup functions for zeroizing aes_cmac_key & aes_cmac_ctx Thomas Huth
                   ` (2 preceding siblings ...)
  2026-08-05 14:36 ` [PATCH 3/6] net/tcp-ao: clear the aes_cmac_key " Thomas Huth
@ 2026-08-05 14:36 ` Thomas Huth
  2026-08-05 20:46   ` Eric Biggers
  2026-08-05 14:36 ` [PATCH 5/6] lib/crypto: aes: Use _cleanup() for aes_cmac_key instead of memzero_explicit() Thomas Huth
  2026-08-05 14:36 ` [PATCH 6/6] mac80211: fils_aead: Use _cleanup for aes_cmac_key instead of memzero_explicit Thomas Huth
  5 siblings, 1 reply; 14+ messages in thread
From: Thomas Huth @ 2026-08-05 14:36 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Eric Biggers, Marcel Holtmann,
	Luiz Augusto von Dentz
  Cc: linux-crypto, linux-kernel, linux-bluetooth

From: Thomas Huth <thuth@redhat.com>

Clear the local aes_cmac_key structure via __cleanup() function
when we're done with it to avoid that sensitive data could leak on
the stack.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 net/bluetooth/smp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
index 031d3022cb1e5..ed30ca0d773f1 100644
--- a/net/bluetooth/smp.c
+++ b/net/bluetooth/smp.c
@@ -164,7 +164,7 @@ static inline void swap_buf(const u8 *src, u8 *dst, size_t len)
 static int smp_aes_cmac(const u8 k[16], const u8 *m, size_t len, u8 mac[16])
 {
 	uint8_t tmp[16], mac_msb[16], msg_msb[CMAC_MSG_MAX];
-	struct aes_cmac_key key;
+	struct aes_cmac_key key __cleanup(aes_cmac_zeroize_key);
 	int err;
 
 	if (len > CMAC_MSG_MAX)
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/6] lib/crypto: aes: Use _cleanup() for aes_cmac_key instead of memzero_explicit()
  2026-08-05 14:36 [PATCH 0/6] crypto: Add __cleanup functions for zeroizing aes_cmac_key & aes_cmac_ctx Thomas Huth
                   ` (3 preceding siblings ...)
  2026-08-05 14:36 ` [PATCH 4/6] Bluetooth: SMP: " Thomas Huth
@ 2026-08-05 14:36 ` Thomas Huth
  2026-08-05 14:36 ` [PATCH 6/6] mac80211: fils_aead: Use _cleanup for aes_cmac_key instead of memzero_explicit Thomas Huth
  5 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2026-08-05 14:36 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Eric Biggers, Jason A. Donenfeld,
	Ard Biesheuvel
  Cc: linux-crypto, linux-kernel

From: Thomas Huth <thuth@redhat.com>

By using __cleanup(aes_cmac_zeroize_key) for clearing the key data,
we can save one line of code here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 lib/crypto/aes.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/lib/crypto/aes.c b/lib/crypto/aes.c
index ca733f15b2a8f..895f8dffa44ae 100644
--- a/lib/crypto/aes.c
+++ b/lib/crypto/aes.c
@@ -720,7 +720,7 @@ EXPORT_SYMBOL_NS_GPL(aes_cbcmac_final, "CRYPTO_INTERNAL");
  */
 static void __init aes_cmac_fips_test(void)
 {
-	struct aes_cmac_key key;
+	struct aes_cmac_key key __cleanup(aes_cmac_zeroize_key);
 	u8 mac[AES_BLOCK_SIZE];
 
 	if (aes_cmac_preparekey(&key, fips_test_key, sizeof(fips_test_key)) !=
@@ -729,7 +729,6 @@ static void __init aes_cmac_fips_test(void)
 	aes_cmac(&key, fips_test_data, sizeof(fips_test_data), mac);
 	if (memcmp(fips_test_aes_cmac_value, mac, sizeof(mac)) != 0)
 		panic("aes: CMAC FIPS self-test failed (wrong MAC)\n");
-	memzero_explicit(&key, sizeof(key));
 }
 #else /* CONFIG_CRYPTO_LIB_AES_CBC_MACS */
 static inline void aes_cmac_fips_test(void)
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 6/6] mac80211: fils_aead: Use _cleanup for aes_cmac_key instead of memzero_explicit
  2026-08-05 14:36 [PATCH 0/6] crypto: Add __cleanup functions for zeroizing aes_cmac_key & aes_cmac_ctx Thomas Huth
                   ` (4 preceding siblings ...)
  2026-08-05 14:36 ` [PATCH 5/6] lib/crypto: aes: Use _cleanup() for aes_cmac_key instead of memzero_explicit() Thomas Huth
@ 2026-08-05 14:36 ` Thomas Huth
  5 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2026-08-05 14:36 UTC (permalink / raw)
  To: Herbert Xu, David S. Miller, Eric Biggers, Johannes Berg
  Cc: linux-crypto, linux-kernel, linux-wireless

From: Thomas Huth <thuth@redhat.com>

By using __cleanup(aes_cmac_zeroize_key) for clearing the key data,
we can save one line of code here.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 net/mac80211/fils_aead.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/mac80211/fils_aead.c b/net/mac80211/fils_aead.c
index d2f4a17eab990..293590976489a 100644
--- a/net/mac80211/fils_aead.c
+++ b/net/mac80211/fils_aead.c
@@ -24,7 +24,7 @@ static int aes_s2v(const u8 *in_key, size_t key_len,
 		   size_t num_elem, const u8 *addr[], size_t len[], u8 *v)
 {
 	u8 d[AES_BLOCK_SIZE], tmp[AES_BLOCK_SIZE] = {};
-	struct aes_cmac_key key;
+	struct aes_cmac_key key __cleanup(aes_cmac_zeroize_key);
 	struct aes_cmac_ctx ctx;
 	size_t i;
 	int res;
@@ -62,7 +62,6 @@ static int aes_s2v(const u8 *in_key, size_t key_len,
 	aes_cmac_update(&ctx, d, AES_BLOCK_SIZE);
 	aes_cmac_final(&ctx, v);
 
-	memzero_explicit(&key, sizeof(key));
 	return 0;
 }
 
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/6] crypto: Provide wrapper functions for zeroizing aes_cmac_key and aes_cmac_ctx
  2026-08-05 14:36 ` [PATCH 1/6] crypto: Provide wrapper functions for zeroizing aes_cmac_key and aes_cmac_ctx Thomas Huth
@ 2026-08-05 20:37   ` Eric Biggers
  2026-08-05 20:46   ` Eric Biggers
  1 sibling, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2026-08-05 20:37 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Herbert Xu, David S. Miller, linux-crypto, linux-kernel

On Wed, Aug 05, 2026 at 04:36:04PM +0200, Thomas Huth wrote:
> crypto: Provide wrapper functions for zeroizing aes_cmac_key and aes_cmac_ctx

    lib/crypto: aes-cmac: Add zeroization functions

> +/**
> + * aes_cmac_zeroize_key - Clear a aes_cmac_key structure

aes_cmac_zeroize_key() - Zeroize an aes_cmac_key structure

> + * @ctx: The location of the key structure that should be zeroized
> + *
> + * Explicitly fills the aes_cmac_key with zeroes. This should be done once
> + * the key is not required anymore to avoid that its contents are leaked
> + * on the stack or heap.

The mention of "heap" is kind of misleading, since normally
kfree_sensitive() would be used in that case.  Maybe add: "Only required
if not using kfree_sensitive()."

> + */
> +static inline void aes_cmac_zeroize_key(struct aes_cmac_key *key)
> +{
> +	memzero_explicit(key, sizeof(*key));
> +}

Also maybe put the function definition right after the definition of
struct aes_cmac_key itself, and likewise for struct aes_cmac_ctx.  Then
they would be closely paired with the corresponding structs.

> /**                                                                              
> * aes_cmac_zeroize_ctx - Clear a aes_cmac_ctx structure   

aes_cmac_zeroize_ctx() - Zeroize an aes_cmac_ctx structure

Could we also get notes in the kerneldoc for aes_cmac_preparekey() and
aes_cmac_init()?  For example:

    "On success, the caller should ensure that the prepared key is
    zeroized at the end of its lifetime, e.g. by calling
    aes_cmac_zeroize_key() or kfree_sensitive()."

and

    "The caller should ensure that the context is zeroized at the end of
    its lifetime, e.g. by calling aes_cmac_final() or
    aes_cmac_zeroize_ctx()."

- Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/6] Bluetooth: SMP: clear the aes_cmac_key when done
  2026-08-05 14:36 ` [PATCH 4/6] Bluetooth: SMP: " Thomas Huth
@ 2026-08-05 20:46   ` Eric Biggers
  2026-08-06 13:00     ` Thomas Huth
  0 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2026-08-05 20:46 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Herbert Xu, David S. Miller, Marcel Holtmann,
	Luiz Augusto von Dentz, linux-crypto, linux-kernel,
	linux-bluetooth

On Wed, Aug 05, 2026 at 04:36:07PM +0200, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> Clear the local aes_cmac_key structure via __cleanup() function
> when we're done with it to avoid that sensitive data could leak on
> the stack.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  net/bluetooth/smp.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
> index 031d3022cb1e5..ed30ca0d773f1 100644
> --- a/net/bluetooth/smp.c
> +++ b/net/bluetooth/smp.c
> @@ -164,7 +164,7 @@ static inline void swap_buf(const u8 *src, u8 *dst, size_t len)
>  static int smp_aes_cmac(const u8 k[16], const u8 *m, size_t len, u8 mac[16])
>  {
>  	uint8_t tmp[16], mac_msb[16], msg_msb[CMAC_MSG_MAX];
> -	struct aes_cmac_key key;
> +	struct aes_cmac_key key __cleanup(aes_cmac_zeroize_key);
>  	int err;
>  
>  	if (len > CMAC_MSG_MAX)

Well, the reason I didn't add a memzero_explicit() here when converting
the code to use the AES-CMAC library is because this same function
already puts the raw key on the stack without zeroizing it.

I guess the __cleanup trick makes zeroizing the struct trivial enough
that we should just do it anyway.  But it is always a bit awkward to be
"fixing" something when the same problem is still present.

Perhaps you'd like to expand this patch a bit to zeroize the other data
too?  (There is the 'tmp' array in this same function of course, but
there may be other places that need "fixing" too.)

- Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/6] crypto: Provide wrapper functions for zeroizing aes_cmac_key and aes_cmac_ctx
  2026-08-05 14:36 ` [PATCH 1/6] crypto: Provide wrapper functions for zeroizing aes_cmac_key and aes_cmac_ctx Thomas Huth
  2026-08-05 20:37   ` Eric Biggers
@ 2026-08-05 20:46   ` Eric Biggers
  1 sibling, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2026-08-05 20:46 UTC (permalink / raw)
  To: Thomas Huth; +Cc: Herbert Xu, David S. Miller, linux-crypto, linux-kernel

On Wed, Aug 05, 2026 at 04:36:04PM +0200, Thomas Huth wrote:
> +/**
> + * aes_cmac_zeroize_key - Clear a aes_cmac_key structure
> + * @ctx: The location of the key structure that should be zeroized

There's also a typo here: @ctx should be @key.

- Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/6] net/tcp-ao: clear the aes_cmac_key when done
  2026-08-05 14:36 ` [PATCH 3/6] net/tcp-ao: clear the aes_cmac_key " Thomas Huth
@ 2026-08-05 21:02   ` Eric Biggers
  2026-08-06 13:06     ` Thomas Huth
  0 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2026-08-05 21:02 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Herbert Xu, David S. Miller, Eric Dumazet, Neal Cardwell,
	Jakub Kicinski, Paolo Abeni, linux-crypto, linux-kernel,
	Kuniyuki Iwashima, Simon Horman, netdev

On Wed, Aug 05, 2026 at 04:36:06PM +0200, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> Clear the local aes_cmac_key structure via __cleanup() function
> when we're done with it to avoid that sensitive data could leak on
> the stack.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  net/ipv4/tcp_ao.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
> index a56bb79e15e0e..12c724fed8a26 100644
> --- a/net/ipv4/tcp_ao.c
> +++ b/net/ipv4/tcp_ao.c
> @@ -141,7 +141,7 @@ void tcp_ao_calc_traffic_key(const struct tcp_ao_key *mkt, u8 *traffic_key,
>  					traffic_key);
>  		return;
>  	case TCP_AO_ALGO_AES_128_CMAC: {
> -		struct aes_cmac_key k;
> +		struct aes_cmac_key k __cleanup(aes_cmac_zeroize_key);
>  
>  		aes_cmac_preparekey(&k, mkt->key, AES_KEYSIZE_128);
>  		aes_cmac(&k, input, input_len, traffic_key);

Similar to the bluetooth patch: This is okay, but it seems the TCP-AO
code has never really tried to do key zeroization, which is why I didn't
include a memzero_explicit() here.  Lots of cases, like the various
traffic key buffers, have never been zeroized and still aren't.

The '__cleanup' trick makes this specific case trivial enough that sure,
it might as well be done anyway.  But it would be nice to have a more
comprehensive patch that actually tried to zeroize all TCP-AO keys.
Otherwise random individual fixes like this trickle in over time and it
takes a lot longer.

- Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/6] smb: clear the aes_cmac_key and aes_cmac_ctx when done
  2026-08-05 14:36 ` [PATCH 2/6] smb: clear the aes_cmac_key and aes_cmac_ctx when done Thomas Huth
@ 2026-08-05 21:12   ` Eric Biggers
  0 siblings, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2026-08-05 21:12 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Herbert Xu, David S. Miller, Steve French, Namjae Jeon,
	linux-crypto, linux-kernel, Paulo Alcantara, Ronnie Sahlberg,
	Shyam Prasad N, Tom Talpey, Bharath SM, Sergey Senozhatsky,
	linux-cifs, samba-technical

On Wed, Aug 05, 2026 at 04:36:05PM +0200, Thomas Huth wrote:
> From: Thomas Huth <thuth@redhat.com>
> 
> Clear the local crypto-related structures via __cleanup() functions
> when we're done with them to avoid that sensitive data could leak on
> the stack.
> 
> Note: cmac_ctx in ksmbd_sign_smb3_pdu() gets cleared in aes_cmac_final()
> already, so this does not need a __cleanup() marker.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  fs/smb/client/smb2transport.c | 4 ++--
>  fs/smb/server/auth.c          | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/smb/client/smb2transport.c b/fs/smb/client/smb2transport.c
> index 1143ee52470a7..d23566da2ac81 100644
> --- a/fs/smb/client/smb2transport.c
> +++ b/fs/smb/client/smb2transport.c
> @@ -464,8 +464,8 @@ smb3_calc_signature(struct smb_rqst *rqst, struct TCP_Server_Info *server)
>  	unsigned char smb3_signature[SMB2_CMACAES_SIZE];
>  	struct kvec *iov = rqst->rq_iov;
>  	struct smb2_hdr *shdr = (struct smb2_hdr *)iov[0].iov_base;
> -	struct aes_cmac_key cmac_key;
> -	struct aes_cmac_ctx cmac_ctx;
> +	struct aes_cmac_key cmac_key __cleanup(aes_cmac_zeroize_key);
> +	struct aes_cmac_ctx cmac_ctx __cleanup(aes_cmac_zeroize_ctx);
>  	struct smb_rqst drqst;
>  	u8 key[SMB3_SIGN_KEY_SIZE];

This is another example of a driver that has never made much attempt at
key zeroization.  Even considering just this function, the raw key is
still on the stack and not zeroized.  But it is not just this function,
e.g. the smb2 code does the same.  So yes, the '__cleanup' trick makes
zeroizing these structs easy enough that we might as well do it anyway,
but it would be nice to try to be a bit more comprehensive.

- Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/6] Bluetooth: SMP: clear the aes_cmac_key when done
  2026-08-05 20:46   ` Eric Biggers
@ 2026-08-06 13:00     ` Thomas Huth
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2026-08-06 13:00 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Herbert Xu, David S. Miller, Marcel Holtmann,
	Luiz Augusto von Dentz, linux-crypto, linux-kernel,
	linux-bluetooth

On 05/08/2026 22.46, Eric Biggers wrote:
> On Wed, Aug 05, 2026 at 04:36:07PM +0200, Thomas Huth wrote:
>> From: Thomas Huth <thuth@redhat.com>
>>
>> Clear the local aes_cmac_key structure via __cleanup() function
>> when we're done with it to avoid that sensitive data could leak on
>> the stack.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   net/bluetooth/smp.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/bluetooth/smp.c b/net/bluetooth/smp.c
>> index 031d3022cb1e5..ed30ca0d773f1 100644
>> --- a/net/bluetooth/smp.c
>> +++ b/net/bluetooth/smp.c
>> @@ -164,7 +164,7 @@ static inline void swap_buf(const u8 *src, u8 *dst, size_t len)
>>   static int smp_aes_cmac(const u8 k[16], const u8 *m, size_t len, u8 mac[16])
>>   {
>>   	uint8_t tmp[16], mac_msb[16], msg_msb[CMAC_MSG_MAX];
>> -	struct aes_cmac_key key;
>> +	struct aes_cmac_key key __cleanup(aes_cmac_zeroize_key);
>>   	int err;
>>   
>>   	if (len > CMAC_MSG_MAX)
> 
> Well, the reason I didn't add a memzero_explicit() here when converting
> the code to use the AES-CMAC library is because this same function
> already puts the raw key on the stack without zeroizing it.
> 
> I guess the __cleanup trick makes zeroizing the struct trivial enough
> that we should just do it anyway.  But it is always a bit awkward to be
> "fixing" something when the same problem is still present.
> 
> Perhaps you'd like to expand this patch a bit to zeroize the other data
> too?  (There is the 'tmp' array in this same function of course, but
> there may be other places that need "fixing" too.)
Sure, I can add a line to clear tmp[] here, too.

And I agree, there are other spots in this file that likely need fixing, 
e.g. smp_e() already clears struct aes_enckey aes, but misses to zeroize its 
tmp[] array, too, that again contains key material, I think?

But I think I'll rather tackle those in a separate patch series, since this 
series here is clearly aimed at aes_cmac_key & aes_cmac_ctx.

  Thomas


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/6] net/tcp-ao: clear the aes_cmac_key when done
  2026-08-05 21:02   ` Eric Biggers
@ 2026-08-06 13:06     ` Thomas Huth
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2026-08-06 13:06 UTC (permalink / raw)
  To: Eric Biggers
  Cc: Herbert Xu, David S. Miller, Eric Dumazet, Neal Cardwell,
	Jakub Kicinski, Paolo Abeni, linux-crypto, linux-kernel,
	Kuniyuki Iwashima, Simon Horman, netdev

On 05/08/2026 23.02, Eric Biggers wrote:
> On Wed, Aug 05, 2026 at 04:36:06PM +0200, Thomas Huth wrote:
>> From: Thomas Huth <thuth@redhat.com>
>>
>> Clear the local aes_cmac_key structure via __cleanup() function
>> when we're done with it to avoid that sensitive data could leak on
>> the stack.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   net/ipv4/tcp_ao.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/net/ipv4/tcp_ao.c b/net/ipv4/tcp_ao.c
>> index a56bb79e15e0e..12c724fed8a26 100644
>> --- a/net/ipv4/tcp_ao.c
>> +++ b/net/ipv4/tcp_ao.c
>> @@ -141,7 +141,7 @@ void tcp_ao_calc_traffic_key(const struct tcp_ao_key *mkt, u8 *traffic_key,
>>   					traffic_key);
>>   		return;
>>   	case TCP_AO_ALGO_AES_128_CMAC: {
>> -		struct aes_cmac_key k;
>> +		struct aes_cmac_key k __cleanup(aes_cmac_zeroize_key);
>>   
>>   		aes_cmac_preparekey(&k, mkt->key, AES_KEYSIZE_128);
>>   		aes_cmac(&k, input, input_len, traffic_key);
> 
> Similar to the bluetooth patch: This is okay, but it seems the TCP-AO
> code has never really tried to do key zeroization, which is why I didn't
> include a memzero_explicit() here.  Lots of cases, like the various
> traffic key buffers, have never been zeroized and still aren't.
> 
> The '__cleanup' trick makes this specific case trivial enough that sure,
> it might as well be done anyway.  But it would be nice to have a more
> comprehensive patch that actually tried to zeroize all TCP-AO keys.
> Otherwise random individual fixes like this trickle in over time and it
> takes a lot longer.
Ok, I'll put this on my TODO list and will send a separate patch - for this 
series here, I'd prefer to limit it to aes_cmac_key / aes_cmac_ctx.

  Thomas


^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2026-08-06 13:06 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05 14:36 [PATCH 0/6] crypto: Add __cleanup functions for zeroizing aes_cmac_key & aes_cmac_ctx Thomas Huth
2026-08-05 14:36 ` [PATCH 1/6] crypto: Provide wrapper functions for zeroizing aes_cmac_key and aes_cmac_ctx Thomas Huth
2026-08-05 20:37   ` Eric Biggers
2026-08-05 20:46   ` Eric Biggers
2026-08-05 14:36 ` [PATCH 2/6] smb: clear the aes_cmac_key and aes_cmac_ctx when done Thomas Huth
2026-08-05 21:12   ` Eric Biggers
2026-08-05 14:36 ` [PATCH 3/6] net/tcp-ao: clear the aes_cmac_key " Thomas Huth
2026-08-05 21:02   ` Eric Biggers
2026-08-06 13:06     ` Thomas Huth
2026-08-05 14:36 ` [PATCH 4/6] Bluetooth: SMP: " Thomas Huth
2026-08-05 20:46   ` Eric Biggers
2026-08-06 13:00     ` Thomas Huth
2026-08-05 14:36 ` [PATCH 5/6] lib/crypto: aes: Use _cleanup() for aes_cmac_key instead of memzero_explicit() Thomas Huth
2026-08-05 14:36 ` [PATCH 6/6] mac80211: fils_aead: Use _cleanup for aes_cmac_key instead of memzero_explicit Thomas Huth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox