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

[David Laight <david.laight.linux@gmail.com>]

On Thu, 29 Oct 2026 10:31:19 -0700
Eric Biggers <ebiggers@kernel.org> wrote:

> 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.

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

David

> 
> - Eric
>