1. Patchew
  2. linux
  3. View series

[PATCH] crypto: Prevent kernel-infoleak in rng_recvmsg

Edward Adam Davis posted 1 patch 1 month, 3 weeks ago 
crypto/jitterentropy-kcapi.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] crypto: Prevent kernel-infoleak in rng_recvmsg
Posted by Edward Adam Davis 1 month, 3 weeks ago 
Initialize the intermediary array member to 0 to prevent the kernel from
leaking uninitialized data to user space.

Reported-by: syzbot+e8bcd7ee3db6cb5cb875@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e8bcd7ee3db6cb5cb875
Tested-by: syzbot+e8bcd7ee3db6cb5cb875@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 crypto/jitterentropy-kcapi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
index c24d4ff2b4a8..9e9e069f55af 100644
--- a/crypto/jitterentropy-kcapi.c
+++ b/crypto/jitterentropy-kcapi.c
@@ -107,7 +107,7 @@ int jent_hash_time(void *hash_state, __u64 time, u8 *addtl,
 {
 	struct shash_desc *hash_state_desc = (struct shash_desc *)hash_state;
 	SHASH_DESC_ON_STACK(desc, hash_state_desc->tfm);
-	u8 intermediary[SHA3_256_DIGEST_SIZE];
+	u8 intermediary[SHA3_256_DIGEST_SIZE] = { 0 };
 	__u64 j = 0;
 	int ret;
 
-- 
2.43.0
Re: [PATCH] crypto: Prevent kernel-infoleak in rng_recvmsg
Posted by Herbert Xu 1 month, 2 weeks ago 
On Sat, Aug 09, 2025 at 05:59:43PM +0800, Edward Adam Davis wrote:
> Initialize the intermediary array member to 0 to prevent the kernel from
> leaking uninitialized data to user space.
> 
> Reported-by: syzbot+e8bcd7ee3db6cb5cb875@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=e8bcd7ee3db6cb5cb875
> Tested-by: syzbot+e8bcd7ee3db6cb5cb875@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
>  crypto/jitterentropy-kcapi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
> index c24d4ff2b4a8..9e9e069f55af 100644
> --- a/crypto/jitterentropy-kcapi.c
> +++ b/crypto/jitterentropy-kcapi.c
> @@ -107,7 +107,7 @@ int jent_hash_time(void *hash_state, __u64 time, u8 *addtl,
>  {
>  	struct shash_desc *hash_state_desc = (struct shash_desc *)hash_state;
>  	SHASH_DESC_ON_STACK(desc, hash_state_desc->tfm);
> -	u8 intermediary[SHA3_256_DIGEST_SIZE];
> +	u8 intermediary[SHA3_256_DIGEST_SIZE] = { 0 };

This is not a leak! The stack memroy is hashed and fed into the
entropy pool.  If you can recover the original kernel memory from
the result, then we have much bigger problems :)

Please find a way to mark this as a false positive.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH] crypto: Prevent kernel-infoleak in rng_recvmsg
Posted by Ard Biesheuvel 1 month, 1 week ago 
(cc Kees)

On Sat, 16 Aug 2025 at 11:17, Herbert Xu <herbert@gondor.apana.org.au> wrote:
>
> On Sat, Aug 09, 2025 at 05:59:43PM +0800, Edward Adam Davis wrote:
> > Initialize the intermediary array member to 0 to prevent the kernel from
> > leaking uninitialized data to user space.
> >
> > Reported-by: syzbot+e8bcd7ee3db6cb5cb875@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=e8bcd7ee3db6cb5cb875
> > Tested-by: syzbot+e8bcd7ee3db6cb5cb875@syzkaller.appspotmail.com
> > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > ---
> >  crypto/jitterentropy-kcapi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
> > index c24d4ff2b4a8..9e9e069f55af 100644
> > --- a/crypto/jitterentropy-kcapi.c
> > +++ b/crypto/jitterentropy-kcapi.c
> > @@ -107,7 +107,7 @@ int jent_hash_time(void *hash_state, __u64 time, u8 *addtl,
> >  {
> >       struct shash_desc *hash_state_desc = (struct shash_desc *)hash_state;
> >       SHASH_DESC_ON_STACK(desc, hash_state_desc->tfm);
> > -     u8 intermediary[SHA3_256_DIGEST_SIZE];
> > +     u8 intermediary[SHA3_256_DIGEST_SIZE] = { 0 };
>
> This is not a leak! The stack memroy is hashed and fed into the
> entropy pool.

Is there still a point to doing this now that the compiler
zero-initializes automatic variables? Or does that not apply to u8
arrays? (asking Kees)
Re: [PATCH] crypto: Prevent kernel-infoleak in rng_recvmsg
Posted by Kees Cook 1 month, 1 week ago 
On Tue, Aug 26, 2025 at 03:51:04PM +0200, Ard Biesheuvel wrote:
> (cc Kees)
> 
> On Sat, 16 Aug 2025 at 11:17, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> >
> > On Sat, Aug 09, 2025 at 05:59:43PM +0800, Edward Adam Davis wrote:
> > > Initialize the intermediary array member to 0 to prevent the kernel from
> > > leaking uninitialized data to user space.
> > >
> > > Reported-by: syzbot+e8bcd7ee3db6cb5cb875@syzkaller.appspotmail.com
> > > Closes: https://syzkaller.appspot.com/bug?extid=e8bcd7ee3db6cb5cb875
> > > Tested-by: syzbot+e8bcd7ee3db6cb5cb875@syzkaller.appspotmail.com
> > > Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> > > ---
> > >  crypto/jitterentropy-kcapi.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
> > > index c24d4ff2b4a8..9e9e069f55af 100644
> > > --- a/crypto/jitterentropy-kcapi.c
> > > +++ b/crypto/jitterentropy-kcapi.c
> > > @@ -107,7 +107,7 @@ int jent_hash_time(void *hash_state, __u64 time, u8 *addtl,
> > >  {
> > >       struct shash_desc *hash_state_desc = (struct shash_desc *)hash_state;
> > >       SHASH_DESC_ON_STACK(desc, hash_state_desc->tfm);
> > > -     u8 intermediary[SHA3_256_DIGEST_SIZE];
> > > +     u8 intermediary[SHA3_256_DIGEST_SIZE] = { 0 };
> >
> > This is not a leak! The stack memroy is hashed and fed into the
> > entropy pool.
> 
> Is there still a point to doing this now that the compiler
> zero-initializes automatic variables? Or does that not apply to u8
> arrays? (asking Kees)

Wait, the jitterentropy _depends_ on "uninitialized" stack contents?
That is not a safe way to get entropy (especially since the stack
contents are likely predictable). But regardless, yes, arrays would be
fully wiped by zero-initialized automatic variables.

To force it to be uninit, use __uninitialized.

-- 
Kees Cook
Re: [PATCH] crypto: Prevent kernel-infoleak in rng_recvmsg
Posted by Herbert Xu 1 month, 2 weeks ago 
On Sat, Aug 16, 2025 at 05:17:01PM +0800, Herbert Xu wrote:
> On Sat, Aug 09, 2025 at 05:59:43PM +0800, Edward Adam Davis wrote:
>
> > diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
> > index c24d4ff2b4a8..9e9e069f55af 100644
> > --- a/crypto/jitterentropy-kcapi.c
> > +++ b/crypto/jitterentropy-kcapi.c
> > @@ -107,7 +107,7 @@ int jent_hash_time(void *hash_state, __u64 time, u8 *addtl,
> >  {
> >  	struct shash_desc *hash_state_desc = (struct shash_desc *)hash_state;
> >  	SHASH_DESC_ON_STACK(desc, hash_state_desc->tfm);
> > -	u8 intermediary[SHA3_256_DIGEST_SIZE];
> > +	u8 intermediary[SHA3_256_DIGEST_SIZE] = { 0 };
> 
> This is not a leak! The stack memroy is hashed and fed into the
> entropy pool.  If you can recover the original kernel memory from
> the result, then we have much bigger problems :)
> 
> Please find a way to mark this as a false positive.

I think kmsan_unpoison_memory is the function that you should call.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
[PATCH V2] crypto: Mark intermediary memory as clean
Posted by Edward Adam Davis 1 month, 2 weeks ago 
This is not a leak! The stack memroy is hashed and fed into the
entropy pool. We can't recover the original kernel memory from it.

Reported-by: syzbot+e8bcd7ee3db6cb5cb875@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e8bcd7ee3db6cb5cb875
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
V1 -> V2: mark it as unpoison

 crypto/jitterentropy-kcapi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
index 1266eb790708..4020a6e41b0e 100644
--- a/crypto/jitterentropy-kcapi.c
+++ b/crypto/jitterentropy-kcapi.c
@@ -117,6 +117,7 @@ int jent_hash_time(void *hash_state, __u64 time, u8 *addtl,
 		pr_warn_ratelimited("Unexpected digest size\n");
 		return -EINVAL;
 	}
+	kmsan_unpoison_memory(intermediary, SHA3_256_DIGEST_SIZE);
 
 	/*
 	 * This loop fills a buffer which is injected into the entropy pool.
-- 
2.43.0
Re: [PATCH V2] crypto: Mark intermediary memory as clean
Posted by Herbert Xu 1 month, 2 weeks ago 
On Sun, Aug 17, 2025 at 06:59:15PM +0800, Edward Adam Davis wrote:
> This is not a leak! The stack memroy is hashed and fed into the
> entropy pool. We can't recover the original kernel memory from it.
> 
> Reported-by: syzbot+e8bcd7ee3db6cb5cb875@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=e8bcd7ee3db6cb5cb875
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> V1 -> V2: mark it as unpoison
> 
>  crypto/jitterentropy-kcapi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
> index 1266eb790708..4020a6e41b0e 100644
> --- a/crypto/jitterentropy-kcapi.c
> +++ b/crypto/jitterentropy-kcapi.c
> @@ -117,6 +117,7 @@ int jent_hash_time(void *hash_state, __u64 time, u8 *addtl,
>  		pr_warn_ratelimited("Unexpected digest size\n");
>  		return -EINVAL;
>  	}
> +	kmsan_unpoison_memory(intermediary, SHA3_256_DIGEST_SIZE);

Please change SHA3_256_DIGEST_SIZE to sizeof(intermediary).

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH V2] crypto: Mark intermediary memory as clean
Posted by Edward Adam Davis 1 month, 2 weeks ago 
On Sun, 17 Aug 2025 19:40:56 +0800, Herbert Xu wrote:
> > diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
> > index 1266eb790708..4020a6e41b0e 100644
> > --- a/crypto/jitterentropy-kcapi.c
> > +++ b/crypto/jitterentropy-kcapi.c
> > @@ -117,6 +117,7 @@ int jent_hash_time(void *hash_state, __u64 time, u8 *addtl,
> >  		pr_warn_ratelimited("Unexpected digest size\n");
> >  		return -EINVAL;
> >  	}
> > +	kmsan_unpoison_memory(intermediary, SHA3_256_DIGEST_SIZE);
> 
> Please change SHA3_256_DIGEST_SIZE to sizeof(intermediary).
Why?
Their values are equal, so why use sizeof to calculate?
Similarly, "if (sizeof(intermediary) != crypto_shash_digestsize(desc->tfm)) {",
why not just use SHA3_256_DIGEST_SIZE?

BR,
Edward
Re: [PATCH V2] crypto: Mark intermediary memory as clean
Posted by Herbert Xu 1 month, 2 weeks ago 
On Mon, Aug 18, 2025 at 08:17:17PM +0800, Edward Adam Davis wrote:
>
> Their values are equal, so why use sizeof to calculate?
> Similarly, "if (sizeof(intermediary) != crypto_shash_digestsize(desc->tfm)) {",
> why not just use SHA3_256_DIGEST_SIZE?

Please be consistent with the existing code, every other place
in the function uses sizeof(intermediary) so your patch is the
odd one out.

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH V2] crypto: Mark intermediary memory as clean
Posted by Edward Adam Davis 1 month, 2 weeks ago 
On Mon, 18 Aug 2025 20:30:29 +0800, Herbert Xu wrote:
> Their values are equal, so why use sizeof to calculate?
> Similarly, "if (sizeof(intermediary) != crypto_shash_digestsize(desc->tfm)) {",
> why not just use SHA3_256_DIGEST_SIZE?
Hi Stephan Mueller, can you explain it?

BR,
Edward
Re: [PATCH V2] crypto: Mark intermediary memory as clean
Posted by Stephan Mueller 1 month, 2 weeks ago 
Am Montag, 18. August 2025, 14:43:36 Mitteleuropäische Sommerzeit schrieb 
Edward Adam Davis:

Hi Edward,

> On Mon, 18 Aug 2025 20:30:29 +0800, Herbert Xu wrote:
> > Their values are equal, so why use sizeof to calculate?
> > Similarly, "if (sizeof(intermediary) !=
> > crypto_shash_digestsize(desc->tfm)) {", why not just use
> > SHA3_256_DIGEST_SIZE?
> 
> Hi Stephan Mueller, can you explain it?

If the question is why using sizeof(intermediary) instead of 
SHA3_256_DIGEST_SIZE, then it is very trivial: I always want to avoid any kind 
of double work. If for any reason the buffer size of intermediary changes, the 
current code only requires *one* location to fix it.

When changing the branching condition to use SHA3_256_DIGEST_SIZE, we would 
have to change *two* locations which is more error-prone than to change one. 
This approach is my common coding style to try to minimize the possibilities 
where inconsistencies can occur.

Ciao
Stephan
[PATCH V3] crypto: Mark intermediary memory as clean
Posted by Edward Adam Davis 1 month, 2 weeks ago 
This is not a leak! The stack memroy is hashed and fed into the
entropy pool. We can't recover the original kernel memory from it.

Reported-by: syzbot+e8bcd7ee3db6cb5cb875@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=e8bcd7ee3db6cb5cb875
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
V1 -> V2: mark it as unpoison
V2 -> V3: replace to sizeof, minimize the possibilities where inconsistencies can occur

 crypto/jitterentropy-kcapi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
index 1266eb790708..4020a6e41b0e 100644
--- a/crypto/jitterentropy-kcapi.c
+++ b/crypto/jitterentropy-kcapi.c
@@ -117,6 +117,7 @@ int jent_hash_time(void *hash_state, __u64 time, u8 *addtl,
 		pr_warn_ratelimited("Unexpected digest size\n");
 		return -EINVAL;
 	}
+	kmsan_unpoison_memory(intermediary, sizeof(intermediary));
 
 	/*
 	 * This loop fills a buffer which is injected into the entropy pool.
-- 
2.43.0
Re: [PATCH V3] crypto: Mark intermediary memory as clean
Posted by Herbert Xu 1 month ago 
On Mon, Aug 18, 2025 at 09:24:17PM +0800, Edward Adam Davis wrote:
> This is not a leak! The stack memroy is hashed and fed into the
> entropy pool. We can't recover the original kernel memory from it.
> 
> Reported-by: syzbot+e8bcd7ee3db6cb5cb875@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=e8bcd7ee3db6cb5cb875
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> V1 -> V2: mark it as unpoison
> V2 -> V3: replace to sizeof, minimize the possibilities where inconsistencies can occur
> 
>  crypto/jitterentropy-kcapi.c | 1 +
>  1 file changed, 1 insertion(+)

Patch applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Re: [PATCH V3] crypto: Mark intermediary memory as clean
Posted by Stephan Mueller 1 month, 2 weeks ago 
Am Montag, 18. August 2025, 15:24:17 Mitteleuropäische Sommerzeit schrieb 
Edward Adam Davis:

Hi Edward,

> This is not a leak! The stack memroy is hashed and fed into the
> entropy pool. We can't recover the original kernel memory from it.
> 
> Reported-by: syzbot+e8bcd7ee3db6cb5cb875@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=e8bcd7ee3db6cb5cb875
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>

Thank you for the patch. Just for the records:

- the intermediary buffer could be initialized to 0 without any effect on the 
Jitter RNG, because all it wants is actually the execution of the Keccak 
operation as part of crypto_shhash_finup.

- the intermediary buffer is inserted into the Jitter RNG state to ensure that 
the compiler cannot optimize away the loop if the intermediary buffer would 
not be used at all

- the intermediary buffer is not credited with any entropy as we only want the 
Keccak operation

- by keeping the intermediary uninitialized, the Jitter RNG may get some 
variations from the uninitialized buffer so that its internal state may 
benefit from it

That said, I am fine with this current patch. But if there is still lingering 
concern, I am equally fine to have it initialized to zero.

Thanks a lot
Stephan

> ---
> V1 -> V2: mark it as unpoison
> V2 -> V3: replace to sizeof, minimize the possibilities where
> inconsistencies can occur
> 
>  crypto/jitterentropy-kcapi.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/crypto/jitterentropy-kcapi.c b/crypto/jitterentropy-kcapi.c
> index 1266eb790708..4020a6e41b0e 100644
> --- a/crypto/jitterentropy-kcapi.c
> +++ b/crypto/jitterentropy-kcapi.c
> @@ -117,6 +117,7 @@ int jent_hash_time(void *hash_state, __u64 time, u8
> *addtl, pr_warn_ratelimited("Unexpected digest size\n");
>  		return -EINVAL;
>  	}
> +	kmsan_unpoison_memory(intermediary, sizeof(intermediary));
> 
>  	/*
>  	 * This loop fills a buffer which is injected into the entropy pool.


Ciao
Stephan

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.