Re: [PATCH] lib/crypto: aes: use memzero_explicit() instead of memset()

[Eric Biggers <ebiggers@kernel.org>]

On Wed, Oct 28, 2026 at 08:56:33PM +0000, David Laight wrote:
> On Wed, 28 Oct 2026 15:51:34 +0000
> Eric Biggers <ebiggers@kernel.org> wrote:
> 
> > On Wed, Oct 28, 2026 at 07:56:36PM +0530, Mohamad Raizudeen wrote:
> > > 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 <raizudeen.kerneldev@gmail.com>
> > > ---
> > >  lib/crypto/aes.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/lib/crypto/aes.c b/lib/crypto/aes.c
> > > index e1c191551736..7814ef525239 100644
> > > --- a/lib/crypto/aes.c
> > > +++ b/lib/crypto/aes.c
> > > @@ -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.

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