lib/crypto: aes: use memzero_explicit() instead of memset()

PATCH Applied Mohamad Raizudeen 3 participants 7 messages Oct 28 – Nov 3

Subsystems (2)

Crypto API Crypto library

lore

Share & tools

lore

Mohamad Raizudeen

author

2026-10-28 14:26 UTC #0 lore raw

PATCH 1/1 lib/crypto: aes: use memzero_explicit() instead of memset()

aes_gcm_decrypt() and aes_ccm_decrypt() use plain memset() to wipe the output buffer when authentication fails. But the compiler can optimize that wipe away, so use memzero_explicit() instead of memset().

This matches every other sensitive wipes in this file, which already uses memzero_explicit() for keystream blocks, tmp buffers, ctx teardowns.

Signed-off-by Mohamad Raizudeen <[email protected]>

---
 lib/crypto/aes.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

lib/crypto/aes.c +2 -2

index e1c191551736..7814ef525239 100644
@@ -1733,7 +1733,7 @@ int aes_gcm_decrypt(u8 *dst, const u8 *src, size_t data_len, const u8 *authtag,
 		 * Clear the inauthentic decrypted data so that callers won't
 		 * receive it even if they fail to correctly handle errors.
 		 */
-		memset(dst, 0, data_len);
+		memzero_explicit(dst, data_len);
 	}
 	return err;
 }

@@ -2074,7 +2074,7 @@ int aes_ccm_decrypt(u8 *dst, const u8 *src, size_t data_len, const u8 *authtag,
 		 * Clear the inauthentic decrypted data so that callers won't
 		 * receive it even if they fail to correctly handle errors.
 		 */
-		memset(dst, 0, data_len);
+		memzero_explicit(dst, data_len);
 	}
 	return err;
 }

Signature · 1 line

2.53.0

Eric Biggers

maintainer

2026-10-28 15:51 UTC #1 lore raw

Quoted 18 lines · Mohamad Raizudeen

aes_gcm_decrypt() and aes_ccm_decrypt() use plain memset() to wipe the output buffer when authentication fails. But the compiler can optimize that wipe away, so use memzero_explicit() instead of memset().
This matches every other sensitive wipes in this file, which already uses memzero_explicit() for keystream blocks, tmp buffers, ctx teardowns.
Signed-off-by Mohamad Raizudeen <[email protected]>
---
 lib/crypto/aes.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
lib/crypto/aes.c +0 -0
index e1c191551736..7814ef525239 100644

lib/crypto/aes.c int aes_gcm_decrypt(u8 *dst, const u8 *src, size_t data_len, const u8 *authtag, · line 1736

Quoted 5 lines

lib/crypto/aes.c +1 -1

@@ -1733,7 +1733,7 @@ int aes_gcm_decrypt(u8 *dst, const u8 *src, size_t data_len, const u8 *authtag,
 		 * Clear the inauthentic decrypted data so that callers won't
 		 * receive it even if they fail to correctly handle errors.
 		 */
-		memset(dst, 0, data_len);
+		memzero_explicit(dst, data_len);

This isn't for "zeroization" (making the data unavailable in ramdumps), though. It's just so the caller doesn't accidentally use the inauthentic data, as the comment right above it states.

- Eric

David Laight

2026-10-28 20:56 UTC #2 lore raw

Quoted 14 lines · On Wed, 28 Oct 2026 15:51:34 +0000 Eric Biggers <[email protected]> wrote:

Quoted 13 lines · Mohamad Raizudeen
aes_gcm_decrypt() and aes_ccm_decrypt() use plain memset() to wipe the output buffer when authentication fails. But the compiler can optimize that wipe away, so use memzero_explicit() instead of memset().
This matches every other sensitive wipes in this file, which already uses memzero_explicit() for keystream blocks, tmp buffers, ctx teardowns.
Signed-off-by Mohamad Raizudeen <[email protected]>
---
 lib/crypto/aes.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Quoted 14 lines

lib/crypto/aes.c int aes_gcm_decrypt(u8 *dst, const u8 *src, size_t data_len, const u8 *authtag, · line 1736
Quoted 10 lines
lib/crypto/aes.c +1 -1
index e1c191551736..7814ef525239 100644
@@ -1733,7 +1733,7 @@ int aes_gcm_decrypt(u8 *dst, const u8 *src, size_t data_len, const u8 *authtag,
 		 * Clear the inauthentic decrypted data so that callers won't
 		 * receive it even if they fail to correctly handle errors.
 		 */
-		memset(dst, 0, data_len);
+		memzero_explicit(dst, data_len);  
This isn't for "zeroization" (making the data unavailable in ramdumps), though. It's just so the caller doesn't accidentally use the inauthentic data, as the comment right above it states.

It is also a parameter so has to be zeroed anyway. memzero_explicit() is used to clear on-stack buffers.

David

Quoted 3 lines

- Eric

Eric Biggers

maintainer

2026-10-29 17:31 UTC #3 lore raw

Quoted 32 lines · David Laight

Quoted 14 lines · On Wed, 28 Oct 2026 15:51:34 +0000 Eric Biggers <[email protected]> wrote:
Quoted 13 lines · Mohamad Raizudeen
aes_gcm_decrypt() and aes_ccm_decrypt() use plain memset() to wipe the output buffer when authentication fails. But the compiler can optimize that wipe away, so use memzero_explicit() instead of memset().
This matches every other sensitive wipes in this file, which already uses memzero_explicit() for keystream blocks, tmp buffers, ctx teardowns.
Signed-off-by Mohamad Raizudeen <[email protected]>
---
 lib/crypto/aes.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Quoted 14 lines
lib/crypto/aes.c int aes_gcm_decrypt(u8 *dst, const u8 *src, size_t data_len, const u8 *authtag, · line 1736
Quoted 10 lines
lib/crypto/aes.c +1 -1
index e1c191551736..7814ef525239 100644
@@ -1733,7 +1733,7 @@ int aes_gcm_decrypt(u8 *dst, const u8 *src, size_t data_len, const u8 *authtag,
 		 * Clear the inauthentic decrypted data so that callers won't
 		 * receive it even if they fail to correctly handle errors.
 		 */
-		memset(dst, 0, data_len);
+		memzero_explicit(dst, data_len);  
This isn't for "zeroization" (making the data unavailable in ramdumps), though. It's just so the caller doesn't accidentally use the inauthentic data, as the comment right above it states.

Quoted 2 lines

It is also a parameter so has to be zeroed anyway. memzero_explicit() is used to clear on-stack buffers.

Well, that assumes link-time optimization is not done. The kernel supports being built with link-time optimization.

I guess the motivation for this patch would be:

(1) Shut up LLMs that think the use of memset() is inconsistent

(2) Maybe the caller really does want "zeroization" for the dst buffer, even on failure -- and this guarantees it for them, even if LTO is enabled, without having to do it themselves. (Though on success, they'll have to do it anyway.)

So I'm tempted to apply this patch anyway.

But the zeroization stuff is getting kind of annoying at this point, when it's unreliable anyway (it's not like we zero out the registers, which can then be spilled to the stack and/or written to the task_struct on context switch) and can even be counterproductive (by forcing data to be written to memory so that memset() can run on it, instead of kept entirely in registers). And Sashiko likes to flag them as Critical issues, which is not at all accurate.

- Eric

Mohamad Raizudeen

author

2026-10-30 03:32 UTC #4 lore raw

Quoted 58 lines · Eric Biggers

Quoted 32 lines · David Laight
Quoted 14 lines · On Wed, 28 Oct 2026 15:51:34 +0000 Eric Biggers <[email protected]> wrote:
Quoted 13 lines · Mohamad Raizudeen
aes_gcm_decrypt() and aes_ccm_decrypt() use plain memset() to wipe the output buffer when authentication fails. But the compiler can optimize that wipe away, so use memzero_explicit() instead of memset().
This matches every other sensitive wipes in this file, which already uses memzero_explicit() for keystream blocks, tmp buffers, ctx teardowns.
Signed-off-by Mohamad Raizudeen <[email protected]>
---
 lib/crypto/aes.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Quoted 14 lines
lib/crypto/aes.c int aes_gcm_decrypt(u8 *dst, const u8 *src, size_t data_len, const u8 *authtag, · line 1736
Quoted 10 lines
lib/crypto/aes.c +1 -1
index e1c191551736..7814ef525239 100644
@@ -1733,7 +1733,7 @@ int aes_gcm_decrypt(u8 *dst, const u8 *src, size_t data_len, const u8 *authtag,
 		 * Clear the inauthentic decrypted data so that callers won't
 		 * receive it even if they fail to correctly handle errors.
 		 */
-		memset(dst, 0, data_len);
+		memzero_explicit(dst, data_len);  
This isn't for "zeroization" (making the data unavailable in ramdumps), though. It's just so the caller doesn't accidentally use the inauthentic data, as the comment right above it states.
Quoted 2 lines
It is also a parameter so has to be zeroed anyway. memzero_explicit() is used to clear on-stack buffers.
Well, that assumes link-time optimization is not done. The kernel supports being built with link-time optimization.
I guess the motivation for this patch would be:
(1) Shut up LLMs that think the use of memset() is inconsistent
(2) Maybe the caller really does want "zeroization" for the dst buffer, even on failure -- and this guarantees it for them, even if LTO is enabled, without having to do it themselves. (Though on success, they'll have to do it anyway.)
So I'm tempted to apply this patch anyway.
But the zeroization stuff is getting kind of annoying at this point, when it's unreliable anyway (it's not like we zero out the registers, which can then be spilled to the stack and/or written to the task_struct on context switch) and can even be counterproductive (by forcing data to be written to memory so that memset() can run on it, instead of kept entirely in registers). And Sashiko likes to flag them as Critical issues, which is not at all accurate.

Quoted 1 line

- Eric

You are right, the confusion was caused by my commit message. I said the compiler would optimize that wipe away without mentioneing LTO, which is the key qualifier. That only holds under LTO, which I should have called out. Yes the comment above the memset() makes clear this is defensive, not zeroization in the rampdump sense. Without LTO th caller is opaque so the store survives it, but with LTO the compiler can potentially eldie it... memzero_explicit() prevents that.

Should I send a v2 with just the commit message updated with LTO?

David Laight

2026-10-30 16:31 UTC #5 lore raw

In reply to Eric Biggers

Quoted 50 lines · On Thu, 29 Oct 2026 10:31:19 -0700 Eric Biggers <[email protected]> wrote:

Quoted 32 lines · David Laight
Quoted 14 lines · On Wed, 28 Oct 2026 15:51:34 +0000 Eric Biggers <[email protected]> wrote:
Quoted 13 lines · Mohamad Raizudeen
aes_gcm_decrypt() and aes_ccm_decrypt() use plain memset() to wipe the output buffer when authentication fails. But the compiler can optimize that wipe away, so use memzero_explicit() instead of memset().
This matches every other sensitive wipes in this file, which already uses memzero_explicit() for keystream blocks, tmp buffers, ctx teardowns.
Signed-off-by Mohamad Raizudeen <[email protected]>
---
 lib/crypto/aes.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
Quoted 14 lines
lib/crypto/aes.c int aes_gcm_decrypt(u8 *dst, const u8 *src, size_t data_len, const u8 *authtag, · line 1736
Quoted 10 lines
lib/crypto/aes.c +1 -1
index e1c191551736..7814ef525239 100644
@@ -1733,7 +1733,7 @@ int aes_gcm_decrypt(u8 *dst, const u8 *src, size_t data_len, const u8 *authtag,
 		 * Clear the inauthentic decrypted data so that callers won't
 		 * receive it even if they fail to correctly handle errors.
 		 */
-		memset(dst, 0, data_len);
+		memzero_explicit(dst, data_len);    
This isn't for "zeroization" (making the data unavailable in ramdumps), though. It's just so the caller doesn't accidentally use the inauthentic data, as the comment right above it states.
Quoted 2 lines
It is also a parameter so has to be zeroed anyway. memzero_explicit() is used to clear on-stack buffers.
Well, that assumes link-time optimization is not done. The kernel supports being built with link-time optimization.
I guess the motivation for this patch would be:
(1) Shut up LLMs that think the use of memset() is inconsistent
(2) Maybe the caller really does want "zeroization" for the dst buffer, even on failure -- and this guarantees it for them, even if LTO is enabled, without having to do it themselves. (Though on success, they'll have to do it anyway.)
So I'm tempted to apply this patch anyway.

Quoted 7 lines

But the zeroization stuff is getting kind of annoying at this point, when it's unreliable anyway (it's not like we zero out the registers, which can then be spilled to the stack and/or written to the task_struct on context switch) and can even be counterproductive (by forcing data to be written to memory so that memset() can run on it, instead of kept entirely in registers). And Sashiko likes to flag them as Critical issues, which is not at all accurate.

OTOH, if someone does a memset() then maybe the compiler should never throw it away?

David

Quoted 3 lines

- Eric

Eric Biggers

maintainer Applied

2026-11-03 23:25 UTC #6 lore raw

In reply to Mohamad Raizudeen

Quoted 4 lines · Mohamad Raizudeen

aes_gcm_decrypt() and aes_ccm_decrypt() use plain memset() to wipe the output buffer when authentication fails. But the compiler can optimize that wipe away, so use memzero_explicit() instead of memset().

Quoted 8 lines

This matches every other sensitive wipes in this file, which already uses memzero_explicit() for keystream blocks, tmp buffers, ctx teardowns.
Signed-off-by Mohamad Raizudeen <[email protected]>
---
 lib/crypto/aes.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Applied to https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/log/?h=libcrypto-next

I added the following note:

[EB: I'm applying this mainly so this doesn't get flagged as an
inconsistency again. The main purpose of the zeroing here is just so the caller doesn't accidentally use the data, which a plain memset() is perfectly fine for. ]

- Eric