Re: [PATCH] crypto: chacha20poly1305 - Fix missing state zeroization in xchacha decrypt

[Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>]

On Sat, Oct 03, 2026 at 06:39:26PM +0200, Eric Biggers wrote:
> On Sat, Oct 03, 2026 at 11:28:20AM +0200, Ard Biesheuvel wrote:
> > 
> > 
> > On Sat, 3 Oct 2026, at 10:57, Eric Biggers wrote:
> > > On Sat, Oct 03, 2026 at 09:34:21AM +0200, Ard Biesheuvel wrote:
> > >> 
> > >> 
> > >> On Sat, 3 Oct 2026, at 08:08, Mohamad Raizudeen wrote:
> > >> > The `__chacha20poly1305_decrypt` function does not zeroize the chacha
> > >> > state, unlike its encrypt counterpart. The regular
> > >> > `chacha20poly1305_decrypt` function handles this by manually calling
> > >> > chacha_zeroize_state(). However, `xchacha20poly1305_decrypt` returns
> > >> > the result directly without clearing the state.
> > >> >
> > >> > This leaves the derived chacha20 subkey on the stack after the function
> > >> > returns. Fix this by storing the return value, calling
> > >> > chacha_zeroize_state() and then returning the result, matching the
> > >> > logic in `chacha20poly1305_decrypt`.
> > >> >
> > >> > Fixes: ed20078b7e333 ("crypto: chacha20poly1305 - import construction 
> > >> > and selftest from Zinc")
> > >> > Cc: stable@vger.kernel.org
> > >> > Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
> > >> > ---
> > >> >  lib/crypto/chacha20poly1305.c | 5 ++++-
> > >> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >> >
> > >> > diff --git a/lib/crypto/chacha20poly1305.c 
> > >> > b/lib/crypto/chacha20poly1305.c
> > >> > index ea42a28f4ff7..03b14d272520 100644
> > >> > --- a/lib/crypto/chacha20poly1305.c
> > >> > +++ b/lib/crypto/chacha20poly1305.c
> > >> > @@ -199,10 +199,13 @@ bool xchacha20poly1305_decrypt(u8 *dst, const u8 
> > >> > *src, const size_t src_len,
> > >> >  			       const u8 key[at_least CHACHA20POLY1305_KEY_SIZE])
> > >> >  {
> > >> >  	struct chacha_state chacha_state;
> > >> > +	bool ret;
> > >> > 
> > >> >  	xchacha_init(&chacha_state, key, nonce);
> > >> > -	return __chacha20poly1305_decrypt(dst, src, src_len, ad, ad_len,
> > >> > +	ret = __chacha20poly1305_decrypt(dst, src, src_len, ad, ad_len,
> > >> >  					  &chacha_state);
> > >> > +	chacha_zeroize_state(&chacha_state);
> > >> > +	return ret;
> > >> >  }
> > >> >  EXPORT_SYMBOL(xchacha20poly1305_decrypt);
> > >> > 
> > >> 
> > >> Wouldn't it be better to move the existing call from chacha20poly1305_decrypt()
> > >> to __chacha20poly1305_decrypt()?
> > >
> > > Since __chacha20poly1305_decrypt() has two return statements that would
> > > need to be considered, I think this patch (which makes each
> > > *chacha_init() clearly paired with chacha_zeroize_state()) is slightly
> > > cleaner.
> > >
> > 
> > Fair enough.
> 
> Actually, looking at the whole file, in the encryption case
> __chacha20poly1305_encrypt() already "takes ownership" of the state and
> handles zeroizing it.  So given that, I think it does make sense for
> __chacha20poly1305_decrypt() to do the same thing, for consistency
> between the encryption and decryption cases.
> 
> Mohamad, could you send out a v2 that does that?
> 
> Also, for the subject prefix, use: "lib/crypto: chacha20poly1305:"
> 
> Thanks!
> 
> - Eric

Hi Eric,

Sure, I will send the v2 soon.

Thanks,
Mohamad Raizudeen