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

[PATCH] crypto: chacha20poly1305 - Fix missing state zeroization in xchacha 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.

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);
 
-- 
2.53.0

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

[Ard Biesheuvel]

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()?

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

[Eric Biggers]

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.

- Eric

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

[Ard Biesheuvel]

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.

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

[Eric Biggers]

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

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

[Mohamad Raizudeen]

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

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

[Jason A. Donenfeld]

On Sat, Oct 03, 2026 at 11:38:26AM +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.
> 
> 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);

In my memory, the reason it's like this is because for WireGuard,
xchapoly is just being used to encrypt a cookie value, which has no
forward secrecy concerns at all. So it doesn't matter if it's left
around on the stack.

Are there other use cases, though, where you think it might matter?

Jason

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

[Mohamad Raizudeen]

On Sat, Oct 03, 2026 at 10:22:00PM +0200, Jason A. Donenfeld wrote:
> On Sat, Oct 03, 2026 at 11:38:26AM +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.
> > 
> > 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);
> 
> In my memory, the reason it's like this is because for WireGuard,
> xchapoly is just being used to encrypt a cookie value, which has no
> forward secrecy concerns at all. So it doesn't matter if it's left
> around on the stack.
> 
> Are there other use cases, though, where you think it might matter?
> 
> Jason

No, none that exist today. The only in-tree callers are the wireguard
cookie code and the kunit test and I agree the cookie has no forward
secrecy concerns. My only thought is that the function is
EXPORT_SYMBOL()ed, so a future caller with a long term key would leave
the derived subkey on the stack and since chacha20poly1305_decrypt()
already wipes the state, having the xchacha variant do the same seemed
like safer default.

Also, Eric requested a v2 that moves the zeroization into the helper
function for consistency with the encrypt path, so I will be sending
that out shortly.

Thanks,
Mohamad Raizudeen