[PATCH v2] lib/crypto: chacha20poly1305: zeroize state in __chacha20poly1305_decrypt

[PATCH v2] lib/crypto: chacha20poly1305: zeroize state in __chacha20poly1305_decrypt

[Mohamad Raizudeen]

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, leaving the derived
chacha20 subkey on the stack.

Fix this by moving the chacha_zeroize_state() call into
__chacha20poly1305_decrypt() itself, so the helper cleans up after
itself just like __chacha20poly1305_encrypt does. This ensures all
callers are secure without needing manual cleanup.

Fixes: ed20078b7e333 ("crypto: chacha20poly1305 - import construction and selftest from Zinc")
Cc: stable@vger.kernel.org
Suggested-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
---
Changes in v2:
- Move chacha_zeroize_state() into __chacha20poly1305_decrypt() as
  suggested by Ard and requested by Eric.
- Update subject prefix to "lib/crypto: chacha20poly1305:"

Link to v1: https://lore.kernel.org/all/20261003060826.7792-1-raizudeen.kerneldev@gmail.com/T/

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

diff --git a/lib/crypto/chacha20poly1305.c b/lib/crypto/chacha20poly1305.c
index ea42a28f4ff7..80904321458b 100644
--- a/lib/crypto/chacha20poly1305.c
+++ b/lib/crypto/chacha20poly1305.c
@@ -137,8 +137,10 @@ __chacha20poly1305_decrypt(u8 *dst, const u8 *src, const size_t src_len,
 		__le64 lens[2];
 	} b;
 
-	if (unlikely(src_len < POLY1305_DIGEST_SIZE))
+	if (unlikely(src_len < POLY1305_DIGEST_SIZE)) {
+		chacha_zeroize_state(chacha_state);
 		return false;
+	}
 
 	chacha20_crypt(chacha_state, b.block0, pad0, sizeof(b.block0));
 	poly1305_init(&poly1305_state, b.block0);
@@ -164,6 +166,7 @@ __chacha20poly1305_decrypt(u8 *dst, const u8 *src, const size_t src_len,
 
 	memzero_explicit(&b, sizeof(b));
 
+	chacha_zeroize_state(chacha_state);
 	return !ret;
 }
 
@@ -186,7 +189,6 @@ bool chacha20poly1305_decrypt(u8 *dst, const u8 *src, const size_t src_len,
 	ret = __chacha20poly1305_decrypt(dst, src, src_len, ad, ad_len,
 					 &chacha_state);
 
-	chacha_zeroize_state(&chacha_state);
 	memzero_explicit(iv, sizeof(iv));
 	memzero_explicit(k, sizeof(k));
 	return ret;
-- 
2.53.0

Re: [PATCH v2] lib/crypto: chacha20poly1305: zeroize state in __chacha20poly1305_decrypt

[Eric Biggers]

On Sun, Oct 04, 2026 at 10:04:13AM +0530, 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, leaving the derived
> chacha20 subkey on the stack.
> 
> Fix this by moving the chacha_zeroize_state() call into
> __chacha20poly1305_decrypt() itself, so the helper cleans up after
> itself just like __chacha20poly1305_encrypt does. This ensures all
> callers are secure without needing manual cleanup.
> 
> Fixes: ed20078b7e333 ("crypto: chacha20poly1305 - import construction and selftest from Zinc")
> Cc: stable@vger.kernel.org
> Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>

Sorry, to nitpick this a bit more:

Can you reword this to clarify that this is an ABI robustness
improvement rather than a fix, since currently the single caller of
xchacha20poly1305_decrypt() in wg_cookie_message_consume() doesn't
require forward secrecy, as mentioned by Jason.  And maybe remove Fixes
and 'Cc stable'.  Otherwise this commit will unnecessarily trigger all
the stable backport and CVE spam.

> diff --git a/lib/crypto/chacha20poly1305.c b/lib/crypto/chacha20poly1305.c
> index ea42a28f4ff7..80904321458b 100644
> --- a/lib/crypto/chacha20poly1305.c
> +++ b/lib/crypto/chacha20poly1305.c
> @@ -137,8 +137,10 @@ __chacha20poly1305_decrypt(u8 *dst, const u8 *src, const size_t src_len,
>  		__le64 lens[2];
>  	} b;
>  
> -	if (unlikely(src_len < POLY1305_DIGEST_SIZE))
> +	if (unlikely(src_len < POLY1305_DIGEST_SIZE)) {
> +		chacha_zeroize_state(chacha_state);
>  		return false;
> +	}

How about we move this length check into the two callers before they
write anything to the state at all?  Then the state would not need to be
zeroized if the length check fails.  Note that
chacha20poly1305_decrypt_sg_inplace() already does it this way.

>  	memzero_explicit(&b, sizeof(b));
>  
> +	chacha_zeroize_state(chacha_state);
>  	return !ret;

Nit: Use the same order and whitespace as
chacha20poly1305_crypt_sg_inplace():

	chacha_zeroize_state(chacha_state);
	memzero_explicit(&b, sizeof(b));

	return !ret;

- Eric

Re: [PATCH v2] lib/crypto: chacha20poly1305: zeroize state in __chacha20poly1305_decrypt

[Mohamad Raizudeen]

On Sun, Oct 04, 2026 at 07:21:55PM +0200, Eric Biggers wrote:
> On Sun, Oct 04, 2026 at 10:04:13AM +0530, 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, leaving the derived
> > chacha20 subkey on the stack.
> > 
> > Fix this by moving the chacha_zeroize_state() call into
> > __chacha20poly1305_decrypt() itself, so the helper cleans up after
> > itself just like __chacha20poly1305_encrypt does. This ensures all
> > callers are secure without needing manual cleanup.
> > 
> > Fixes: ed20078b7e333 ("crypto: chacha20poly1305 - import construction and selftest from Zinc")
> > Cc: stable@vger.kernel.org
> > Suggested-by: Ard Biesheuvel <ardb@kernel.org>
> > Signed-off-by: Mohamad Raizudeen <raizudeen.kerneldev@gmail.com>
> 
> Sorry, to nitpick this a bit more:
> 
> Can you reword this to clarify that this is an ABI robustness
> improvement rather than a fix, since currently the single caller of
> xchacha20poly1305_decrypt() in wg_cookie_message_consume() doesn't
> require forward secrecy, as mentioned by Jason.  And maybe remove Fixes
> and 'Cc stable'.  Otherwise this commit will unnecessarily trigger all
> the stable backport and CVE spam.
> 
> > diff --git a/lib/crypto/chacha20poly1305.c b/lib/crypto/chacha20poly1305.c
> > index ea42a28f4ff7..80904321458b 100644
> > --- a/lib/crypto/chacha20poly1305.c
> > +++ b/lib/crypto/chacha20poly1305.c
> > @@ -137,8 +137,10 @@ __chacha20poly1305_decrypt(u8 *dst, const u8 *src, const size_t src_len,
> >  		__le64 lens[2];
> >  	} b;
> >  
> > -	if (unlikely(src_len < POLY1305_DIGEST_SIZE))
> > +	if (unlikely(src_len < POLY1305_DIGEST_SIZE)) {
> > +		chacha_zeroize_state(chacha_state);
> >  		return false;
> > +	}
> 
> How about we move this length check into the two callers before they
> write anything to the state at all?  Then the state would not need to be
> zeroized if the length check fails.  Note that
> chacha20poly1305_decrypt_sg_inplace() already does it this way.
> 
> >  	memzero_explicit(&b, sizeof(b));
> >  
> > +	chacha_zeroize_state(chacha_state);
> >  	return !ret;
> 
> Nit: Use the same order and whitespace as
> chacha20poly1305_crypt_sg_inplace():
> 
> 	chacha_zeroize_state(chacha_state);
> 	memzero_explicit(&b, sizeof(b));
> 
> 	return !ret;
> 
> - Eric

Sure, that makes sense. I will drop the fixes and stable tags and reword
the commit message to focus on ABI robustness.

Moving the length check to the callers is a much cleaner approach. I
will implement that and fix the whitespace ordering for v3.

Thanks,
Mohamad Raizudeen