* Re: [PATCH] crypto: sun4i-ss: restrict PRNG seed length to prevent heap overflow
2026-05-28 14:53 [PATCH] crypto: sun4i-ss: restrict PRNG seed length to prevent heap overflow Tianchu Chen
@ 2026-05-28 15:29 ` sashiko-bot
2026-05-29 3:03 ` Tianchu Chen
2026-05-29 6:11 ` Herbert Xu
2026-05-29 8:08 ` [PATCH v2] crypto: sun4i-ss - clamp " Tianchu Chen
2 siblings, 1 reply; 13+ messages in thread
From: sashiko-bot @ 2026-05-28 15:29 UTC (permalink / raw)
To: Tianchu Chen; +Cc: linux-sunxi
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [Critical] The newly added bounds check unconditionally rejects valid seeds and completely breaks PRNG initialization on 64-bit architectures.
Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but the PRNG generation function silently truncates unaligned requested lengths, failing to generate random bytes for the remainder of the buffer while still returning success.
- [High] This problem wasn't introduced by this patch, but the patch silently permits undersized seeds, leading to partial seed updates.
- [High] This problem wasn't introduced by this patch, but the PRNG seed is copied into the shared device state without any locking, creating a data race with concurrent PRNG generations.
- [High] This problem wasn't introduced by this patch, but there is a blind read from TXFIFO in PRNG generation without checking if data is available.
--
commit ce68b95827dcfbd778d1a5a3d2de5fd10e821954
Author: Tianchu Chen <flynnnchen@tencent.com>
crypto: sun4i-ss: restrict PRNG seed length to prevent heap overflow
This commit restricts the PRNG seed length in sun4i_ss_prng_seed() to prevent
a heap overflow. Without bounds checking, user-supplied seeds via AF_ALG could
exceed the fixed-size buffer and corrupt adjacent memory.
Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
Cc: stable@vger.kernel.org
> diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
> index 491fcb7b81b40..010fa891c1056 100644
> --- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
> +++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
> @@ -8,6 +8,8 @@ int sun4i_ss_prng_seed(struct crypto_rng *tfm, const u8 *seed,
> struct rng_alg *alg = crypto_rng_alg(tfm);
>
> algt = container_of(alg, struct sun4i_ss_alg_template, alg.rng);
> + if (slen > sizeof(algt->ss->seed))
> + return -EINVAL;
[Severity: Critical]
Does sizeof(algt->ss->seed) unconditionally reject valid seeds?
On 64-bit architectures, BITS_PER_LONG is 64, which makes the evaluation of
SS_SEED_LEN / BITS_PER_LONG result in 3. The seed array becomes u32 seed[3],
yielding a total size of 12 bytes. This would trigger the EINVAL branch for
standard 24-byte seeds and completely break PRNG initialization.
[Severity: High]
This is a pre-existing issue, but does this check silently permit undersized
seeds?
Allowing slen < seedsize enables partial memcpy operations that mix stale
memory with new entropy.
> memcpy(algt->ss->seed, seed, slen);
[Severity: High]
This isn't a bug introduced by this patch, but could this copy operation
create a data race?
The PRNG seed is copied into the shared device state without any locking,
which could lead to torn reads or writes with concurrent PRNG generations.
>
> return 0;
> }
[Severity: Critical]
This problem wasn't introduced by this patch, but are unaligned requested
lengths silently truncated by the generation function?
Looking at sun4i_ss_prng_generate() in the same file:
unsigned int todo = (dlen / 4) * 4;
...
while (todo > 0) {
...
}
The unaligned modulo truncation skips remainder bytes while still returning
success. This fails to generate random bytes for the remainder of the buffer
and could potentially leak uninitialized memory to userspace.
[Severity: High]
This is a pre-existing issue, but does the generator perform a blind read
from the TXFIFO?
Looking at sun4i_ss_prng_generate() in the same file:
/* Read the random data */
len = min_t(size_t, SS_DATA_LEN / BITS_PER_BYTE, todo);
readsl(ss->base + SS_TXFIFO, data, len / 4);
This reads from the TXFIFO without checking if data is actually available,
which could fetch unready values before hardware generation completes and
result in predictable or garbage PRNG output.
--
Sashiko AI review · https://sashiko.dev/#/patchset/af749a8447bd7f0e9dd26ca6c87e9c6afecb09d9@linux.dev?part=1
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH] crypto: sun4i-ss: restrict PRNG seed length to prevent heap overflow
2026-05-28 15:29 ` sashiko-bot
@ 2026-05-29 3:03 ` Tianchu Chen
0 siblings, 0 replies; 13+ messages in thread
From: Tianchu Chen @ 2026-05-29 3:03 UTC (permalink / raw)
To: sashiko-reviews; +Cc: linux-sunxi
May 28, 2026 at 11:29 PM, sashiko-bot@kernel.org mailto:sashiko-bot@kernel.org wrote:
>
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
>
> New issues:
> - [Critical] The newly added bounds check unconditionally rejects valid seeds and completely breaks PRNG initialization on 64-bit architectures.
>
> Pre-existing issues:
> - [Critical] This problem wasn't introduced by this patch, but the PRNG generation function silently truncates unaligned requested lengths, failing to generate random bytes for the remainder of the buffer while still returning success.
> - [High] This problem wasn't introduced by this patch, but the patch silently permits undersized seeds, leading to partial seed updates.
> - [High] This problem wasn't introduced by this patch, but the PRNG seed is copied into the shared device state without any locking, creating a data race with concurrent PRNG generations.
> - [High] This problem wasn't introduced by this patch, but there is a blind read from TXFIFO in PRNG generation without checking if data is available.
> --
>
> commit ce68b95827dcfbd778d1a5a3d2de5fd10e821954
> Author: Tianchu Chen <flynnnchen@tencent.com>
> crypto: sun4i-ss: restrict PRNG seed length to prevent heap overflow
>
> This commit restricts the PRNG seed length in sun4i_ss_prng_seed() to prevent
> a heap overflow. Without bounds checking, user-supplied seeds via AF_ALG could
> exceed the fixed-size buffer and corrupt adjacent memory.
>
> Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
> Cc: stable@vger.kernel.org
>
> >
> > diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
> > index 491fcb7b81b40..010fa891c1056 100644
> > --- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
> > +++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
> > @@ -8,6 +8,8 @@ int sun4i_ss_prng_seed(struct crypto_rng *tfm, const u8 *seed,
> > struct rng_alg *alg = crypto_rng_alg(tfm);
> >
> > algt = container_of(alg, struct sun4i_ss_alg_template, alg.rng);
> > + if (slen > sizeof(algt->ss->seed))
> > + return -EINVAL;
> >
> [Severity: Critical]
> Does sizeof(algt->ss->seed) unconditionally reject valid seeds?
>
> On 64-bit architectures, BITS_PER_LONG is 64, which makes the evaluation of
> SS_SEED_LEN / BITS_PER_LONG result in 3. The seed array becomes u32 seed[3],
> yielding a total size of 12 bytes. This would trigger the EINVAL branch for
> standard 24-byte seeds and completely break PRNG initialization.
>
sun4i-ss is designed for 32-bit ARM only (A10/A13/A20/A33). It's behind
ARCH_SUNXI in Kconfig so it never builds on 64-bit. On the real hardware
sizeof(ss->seed) is always 24, same as .seedsize.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] crypto: sun4i-ss: restrict PRNG seed length to prevent heap overflow
2026-05-28 14:53 [PATCH] crypto: sun4i-ss: restrict PRNG seed length to prevent heap overflow Tianchu Chen
2026-05-28 15:29 ` sashiko-bot
@ 2026-05-29 6:11 ` Herbert Xu
2026-05-29 7:05 ` Tianchu Chen
2026-05-29 8:08 ` [PATCH v2] crypto: sun4i-ss - clamp " Tianchu Chen
2 siblings, 1 reply; 13+ messages in thread
From: Herbert Xu @ 2026-05-29 6:11 UTC (permalink / raw)
To: Tianchu Chen
Cc: clabbe.montjoie, davem, wens, jernej.skrabec, samuel,
linux-crypto, linux-arm-kernel, linux-sunxi, linux-kernel
On Thu, May 28, 2026 at 02:53:17PM +0000, Tianchu Chen wrote:
>
> diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
> index 491fcb7b8..010fa891c 100644
> --- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
> +++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
> @@ -8,6 +8,8 @@ int sun4i_ss_prng_seed(struct crypto_rng *tfm, const u8 *seed,
> struct rng_alg *alg = crypto_rng_alg(tfm);
>
> algt = container_of(alg, struct sun4i_ss_alg_template, alg.rng);
> + if (slen > sizeof(algt->ss->seed))
> + return -EINVAL;
This should simply ignore the extra data instead of failing.
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH] crypto: sun4i-ss: restrict PRNG seed length to prevent heap overflow
2026-05-29 6:11 ` Herbert Xu
@ 2026-05-29 7:05 ` Tianchu Chen
0 siblings, 0 replies; 13+ messages in thread
From: Tianchu Chen @ 2026-05-29 7:05 UTC (permalink / raw)
To: Herbert Xu
Cc: clabbe.montjoie, davem, wens, jernej.skrabec, samuel,
linux-crypto, linux-arm-kernel, linux-sunxi, linux-kernel
May 29, 2026 at 2:11 PM, "Herbert Xu" <herbert@gondor.apana.org.au mailto:herbert@gondor.apana.org.au?to=%22Herbert%20Xu%22%20%3Cherbert%40gondor.apana.org.au%3E > wrote:
>
> On Thu, May 28, 2026 at 02:53:17PM +0000, Tianchu Chen wrote:
>
> >
> > diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
> > index 491fcb7b8..010fa891c 100644
> > --- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
> > +++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
> > @@ -8,6 +8,8 @@ int sun4i_ss_prng_seed(struct crypto_rng *tfm, const u8 *seed,
> > struct rng_alg *alg = crypto_rng_alg(tfm);
> >
> > algt = container_of(alg, struct sun4i_ss_alg_template, alg.rng);
> > + if (slen > sizeof(algt->ss->seed))
> > + return -EINVAL;
> >
> This should simply ignore the extra data instead of failing.
Thanks for pointing out, silent truncation is more appropriate here.
I'll send a v2 patch with min_t soon.
Best regards,
Tianchu Chen
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2] crypto: sun4i-ss - clamp PRNG seed length to prevent heap overflow
2026-05-28 14:53 [PATCH] crypto: sun4i-ss: restrict PRNG seed length to prevent heap overflow Tianchu Chen
2026-05-28 15:29 ` sashiko-bot
2026-05-29 6:11 ` Herbert Xu
@ 2026-05-29 8:08 ` Tianchu Chen
2026-05-29 8:42 ` sashiko-bot
2026-05-29 16:10 ` Eric Biggers
2 siblings, 2 replies; 13+ messages in thread
From: Tianchu Chen @ 2026-05-29 8:08 UTC (permalink / raw)
To: clabbe.montjoie, herbert, davem
Cc: wens, jernej.skrabec, samuel, linux-crypto, linux-arm-kernel,
linux-sunxi, linux-kernel
From: Tianchu Chen <flynnnchen@tencent.com>
sun4i_ss_prng_seed() copies the user-supplied seed into ss->seed
using the user-provided length with no bounds check. The crypto core
does not enforce slen <= seedsize before calling into the driver, so a
userspace caller via AF_ALG setsockopt(ALG_SET_KEY) can pass up to
sysctl_optmem_max bytes, overflowing the fixed-size buffer and
corrupting adjacent heap memory.
Clamp the copy length to the buffer size, matching the approach used by
loongson-rng for oversized seeds.
Discovered by Atuin - Automated Vulnerability Discovery Engine.
Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
Cc: stable@vger.kernel.org
Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
---
v2: Silently clamp oversized seeds with min_t instead of returning
-EINVAL (Herbert Xu).
drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
index 491fcb7b8..7f6a51dd8 100644
--- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
+++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
@@ -8,7 +8,7 @@ int sun4i_ss_prng_seed(struct crypto_rng *tfm, const u8 *seed,
struct rng_alg *alg = crypto_rng_alg(tfm);
algt = container_of(alg, struct sun4i_ss_alg_template, alg.rng);
- memcpy(algt->ss->seed, seed, slen);
+ memcpy(algt->ss->seed, seed, min_t(unsigned int, slen, sizeof(algt->ss->seed)));
return 0;
}
--
2.51.0
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2] crypto: sun4i-ss - clamp PRNG seed length to prevent heap overflow
2026-05-29 8:08 ` [PATCH v2] crypto: sun4i-ss - clamp " Tianchu Chen
@ 2026-05-29 8:42 ` sashiko-bot
2026-05-29 16:10 ` Eric Biggers
1 sibling, 0 replies; 13+ messages in thread
From: sashiko-bot @ 2026-05-29 8:42 UTC (permalink / raw)
To: Tianchu Chen; +Cc: linux-sunxi
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
Pre-existing issues:
- [Critical] The PRNG hardware state is shared globally across all crypto API instances, allowing unprivileged users to predictably control the RNG state for others.
- [High] The PRNG seed buffer size incorrectly depends on BITS_PER_LONG, causing seed truncation and halving entropy on 64-bit systems.
- [High] sun4i_ss_prng_seed modifies the shared PRNG seed buffer without taking the required device spinlock.
- [High] The driver fails to validate that the provided seed length is sufficient before updating the PRNG seed state.
- [High] sun4i_ss_prng_generate silently ignores trailing bytes when the requested length is not a multiple of 4, leaving parts of the caller's buffer uninitialized.
--
commit c4469644d7be8a19b75d7ee7a736e9f34531be57
Author: Tianchu Chen <flynnnchen@tencent.com>
crypto: sun4i-ss - clamp PRNG seed length to prevent heap overflow
This commit clamps the copy length in sun4i_ss_prng_seed() to prevent a
heap overflow when a user supplies an oversized seed via AF_ALG. It uses
min_t to restrict the length to the size of the destination seed buffer.
> diff --git a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
> index 491fcb7b81b40..7f6a51dd8f923 100644
> --- a/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
> +++ b/drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c
> @@ -8,7 +8,7 @@ int sun4i_ss_prng_seed(struct crypto_rng *tfm, const u8 *seed,
> struct rng_alg *alg = crypto_rng_alg(tfm);
>
> algt = container_of(alg, struct sun4i_ss_alg_template, alg.rng);
[Severity: Critical]
This is a pre-existing issue, but the PRNG hardware state is shared globally
across all crypto API instances. Since there is no per-tfm context and the
driver registers as stdrng with priority 300, can an unprivileged user open
an AF_ALG socket and use setsockopt to inject a chosen seed here? This would
allow them to predict the sequence of random numbers generated for other
processes or kernel subsystems relying on the same RNG.
> - memcpy(algt->ss->seed, seed, slen);
[Severity: High]
This is a pre-existing issue, but does this function modify the shared PRNG
seed buffer without taking the required device spinlock? In
sun4i_ss_prng_generate(), the code acquires spin_lock_bh(&ss->slock) before
accessing the buffer. Concurrent calls to sun4i_ss_prng_seed() and generate
could cause a data race resulting in torn state.
> + memcpy(algt->ss->seed, seed, min_t(unsigned int, slen, sizeof(algt->ss->seed)));
[Severity: High]
This is a pre-existing issue, but does the seed buffer size incorrectly
depend on BITS_PER_LONG? If the driver defines the seed array using
SS_SEED_LEN / BITS_PER_LONG, on 64-bit systems this becomes an array of 3
elements (12 bytes) instead of the expected 24 bytes. This would mean the
sizeof() calculation silently truncates valid 24-byte seeds to 12 bytes and
fails to initialize half of the PRNG hardware seed registers.
>
> return 0;
[Severity: High]
This is a pre-existing issue, but does this function fail to validate that
the provided seed length is sufficient? If a caller provides an undersized
seed, the memcpy will partially update the array, but the function will
still unconditionally return 0. Should the code return an error when the
seed is too small instead of signaling a successful reseeding?
> }
[Severity: High]
This is a pre-existing issue, but looking at sun4i_ss_prng_generate(), if
the requested length is not a multiple of 4, the remaining bytes are
silently ignored while still returning 0 (success). Does this leave portions
of the caller's buffer completely uninitialized, potentially causing
information disclosure or cryptographic vulnerabilities if predictable memory
is used as key material?
--
Sashiko AI review · https://sashiko.dev/#/patchset/4d4407c05835a50413fa1e974e3aa3f4abfe2d5b@linux.dev?part=1
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2] crypto: sun4i-ss - clamp PRNG seed length to prevent heap overflow
2026-05-29 8:08 ` [PATCH v2] crypto: sun4i-ss - clamp " Tianchu Chen
2026-05-29 8:42 ` sashiko-bot
@ 2026-05-29 16:10 ` Eric Biggers
2026-05-29 17:10 ` Corentin Labbe
1 sibling, 1 reply; 13+ messages in thread
From: Eric Biggers @ 2026-05-29 16:10 UTC (permalink / raw)
To: Tianchu Chen
Cc: clabbe.montjoie, herbert, davem, wens, jernej.skrabec, samuel,
linux-crypto, linux-arm-kernel, linux-sunxi, linux-kernel
On Fri, May 29, 2026 at 08:08:01AM +0000, Tianchu Chen wrote:
> From: Tianchu Chen <flynnnchen@tencent.com>
>
> sun4i_ss_prng_seed() copies the user-supplied seed into ss->seed
> using the user-provided length with no bounds check. The crypto core
> does not enforce slen <= seedsize before calling into the driver, so a
> userspace caller via AF_ALG setsockopt(ALG_SET_KEY) can pass up to
> sysctl_optmem_max bytes, overflowing the fixed-size buffer and
> corrupting adjacent heap memory.
>
> Clamp the copy length to the buffer size, matching the approach used by
> loongson-rng for oversized seeds.
>
> Discovered by Atuin - Automated Vulnerability Discovery Engine.
>
> Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
> ---
> v2: Silently clamp oversized seeds with min_t instead of returning
> -EINVAL (Herbert Xu).
sun4i-ss-prng.c is useless, is still broken, and should just be deleted.
- Eric
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] crypto: sun4i-ss - clamp PRNG seed length to prevent heap overflow
2026-05-29 16:10 ` Eric Biggers
@ 2026-05-29 17:10 ` Corentin Labbe
2026-05-29 17:33 ` Eric Biggers
0 siblings, 1 reply; 13+ messages in thread
From: Corentin Labbe @ 2026-05-29 17:10 UTC (permalink / raw)
To: Eric Biggers
Cc: Tianchu Chen, herbert, davem, wens, jernej.skrabec, samuel,
linux-crypto, linux-arm-kernel, linux-sunxi, linux-kernel
Le Fri, May 29, 2026 at 09:10:57AM -0700, Eric Biggers a écrit :
> On Fri, May 29, 2026 at 08:08:01AM +0000, Tianchu Chen wrote:
> > From: Tianchu Chen <flynnnchen@tencent.com>
> >
> > sun4i_ss_prng_seed() copies the user-supplied seed into ss->seed
> > using the user-provided length with no bounds check. The crypto core
> > does not enforce slen <= seedsize before calling into the driver, so a
> > userspace caller via AF_ALG setsockopt(ALG_SET_KEY) can pass up to
> > sysctl_optmem_max bytes, overflowing the fixed-size buffer and
> > corrupting adjacent heap memory.
> >
> > Clamp the copy length to the buffer size, matching the approach used by
> > loongson-rng for oversized seeds.
> >
> > Discovered by Atuin - Automated Vulnerability Discovery Engine.
> >
> > Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
> > ---
> > v2: Silently clamp oversized seeds with min_t instead of returning
> > -EINVAL (Herbert Xu).
>
> sun4i-ss-prng.c is useless, is still broken, and should just be deleted.
Hello
useless ? clearly no, it helped a lot on devices where it is.
Regards
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] crypto: sun4i-ss - clamp PRNG seed length to prevent heap overflow
2026-05-29 17:10 ` Corentin Labbe
@ 2026-05-29 17:33 ` Eric Biggers
2026-05-29 19:41 ` Eric Biggers
2026-06-01 14:57 ` Corentin Labbe
0 siblings, 2 replies; 13+ messages in thread
From: Eric Biggers @ 2026-05-29 17:33 UTC (permalink / raw)
To: Corentin Labbe
Cc: Tianchu Chen, herbert, davem, wens, jernej.skrabec, samuel,
linux-crypto, linux-arm-kernel, linux-sunxi, linux-kernel
On Fri, May 29, 2026 at 07:10:06PM +0200, Corentin Labbe wrote:
> Le Fri, May 29, 2026 at 09:10:57AM -0700, Eric Biggers a écrit :
> > On Fri, May 29, 2026 at 08:08:01AM +0000, Tianchu Chen wrote:
> > > From: Tianchu Chen <flynnnchen@tencent.com>
> > >
> > > sun4i_ss_prng_seed() copies the user-supplied seed into ss->seed
> > > using the user-provided length with no bounds check. The crypto core
> > > does not enforce slen <= seedsize before calling into the driver, so a
> > > userspace caller via AF_ALG setsockopt(ALG_SET_KEY) can pass up to
> > > sysctl_optmem_max bytes, overflowing the fixed-size buffer and
> > > corrupting adjacent heap memory.
> > >
> > > Clamp the copy length to the buffer size, matching the approach used by
> > > loongson-rng for oversized seeds.
> > >
> > > Discovered by Atuin - Automated Vulnerability Discovery Engine.
> > >
> > > Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
> > > ---
> > > v2: Silently clamp oversized seeds with min_t instead of returning
> > > -EINVAL (Herbert Xu).
> >
> > sun4i-ss-prng.c is useless, is still broken, and should just be deleted.
>
> Hello
>
> useless ? clearly no, it helped a lot on devices where it is.
The only way this code is reachable is via "rng" algorithm type in
AF_ALG, which is almost never used. Everyone just uses the regular
Linux RNG (/dev/random etc) instead, as they should.
In fact, anyone were to accidentally use this it would be a security
vulnerability, seeing as sun4i_ss_prng_generate() doesn't actually fill
in all the bytes that were requested. It also doesn't wait for the FIFO
to be ready when reading data from it.
Is it possible that there's a misunderstanding here and you think this
provides entropy to the regular Linux RNG? It doesn't. hwrng does
that, crypto_rng does not.
The correct fix is to mark CRYPTO_DEV_SUN8I_CE_PRNG as BROKEN or remove
it entirely. Doing otherwise is not responsible.
- Eric
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] crypto: sun4i-ss - clamp PRNG seed length to prevent heap overflow
2026-05-29 17:33 ` Eric Biggers
@ 2026-05-29 19:41 ` Eric Biggers
2026-06-01 14:57 ` Corentin Labbe
1 sibling, 0 replies; 13+ messages in thread
From: Eric Biggers @ 2026-05-29 19:41 UTC (permalink / raw)
To: Corentin Labbe
Cc: Tianchu Chen, herbert, davem, wens, jernej.skrabec, samuel,
linux-crypto, linux-arm-kernel, linux-sunxi, linux-kernel
On Fri, May 29, 2026 at 05:33:41PM +0000, Eric Biggers wrote:
> On Fri, May 29, 2026 at 07:10:06PM +0200, Corentin Labbe wrote:
> > Le Fri, May 29, 2026 at 09:10:57AM -0700, Eric Biggers a écrit :
> > > On Fri, May 29, 2026 at 08:08:01AM +0000, Tianchu Chen wrote:
> > > > From: Tianchu Chen <flynnnchen@tencent.com>
> > > >
> > > > sun4i_ss_prng_seed() copies the user-supplied seed into ss->seed
> > > > using the user-provided length with no bounds check. The crypto core
> > > > does not enforce slen <= seedsize before calling into the driver, so a
> > > > userspace caller via AF_ALG setsockopt(ALG_SET_KEY) can pass up to
> > > > sysctl_optmem_max bytes, overflowing the fixed-size buffer and
> > > > corrupting adjacent heap memory.
> > > >
> > > > Clamp the copy length to the buffer size, matching the approach used by
> > > > loongson-rng for oversized seeds.
> > > >
> > > > Discovered by Atuin - Automated Vulnerability Discovery Engine.
> > > >
> > > > Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
> > > > ---
> > > > v2: Silently clamp oversized seeds with min_t instead of returning
> > > > -EINVAL (Herbert Xu).
> > >
> > > sun4i-ss-prng.c is useless, is still broken, and should just be deleted.
> >
> > Hello
> >
> > useless ? clearly no, it helped a lot on devices where it is.
>
> The only way this code is reachable is via "rng" algorithm type in
> AF_ALG, which is almost never used. Everyone just uses the regular
> Linux RNG (/dev/random etc) instead, as they should.
>
> In fact, anyone were to accidentally use this it would be a security
> vulnerability, seeing as sun4i_ss_prng_generate() doesn't actually fill
> in all the bytes that were requested. It also doesn't wait for the FIFO
> to be ready when reading data from it.
>
> Is it possible that there's a misunderstanding here and you think this
> provides entropy to the regular Linux RNG? It doesn't. hwrng does
> that, crypto_rng does not.
>
> The correct fix is to mark CRYPTO_DEV_SUN8I_CE_PRNG as BROKEN or remove
> it entirely. Doing otherwise is not responsible.
Looking into it a bit more, just removing CRYPTO_DEV_SUN4I_SS_PRNG is
clearly the way to go. This patch does it:
https://lore.kernel.org/linux-crypto/20260529193648.18172-1-ebiggers@kernel.org
- Eric
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] crypto: sun4i-ss - clamp PRNG seed length to prevent heap overflow
2026-05-29 17:33 ` Eric Biggers
2026-05-29 19:41 ` Eric Biggers
@ 2026-06-01 14:57 ` Corentin Labbe
2026-06-01 15:47 ` Eric Biggers
1 sibling, 1 reply; 13+ messages in thread
From: Corentin Labbe @ 2026-06-01 14:57 UTC (permalink / raw)
To: Eric Biggers
Cc: Tianchu Chen, herbert, davem, wens, jernej.skrabec, samuel,
linux-crypto, linux-arm-kernel, linux-sunxi, linux-kernel
Le Fri, May 29, 2026 at 05:33:41PM +0000, Eric Biggers a écrit :
> On Fri, May 29, 2026 at 07:10:06PM +0200, Corentin Labbe wrote:
> > Le Fri, May 29, 2026 at 09:10:57AM -0700, Eric Biggers a écrit :
> > > On Fri, May 29, 2026 at 08:08:01AM +0000, Tianchu Chen wrote:
> > > > From: Tianchu Chen <flynnnchen@tencent.com>
> > > >
> > > > sun4i_ss_prng_seed() copies the user-supplied seed into ss->seed
> > > > using the user-provided length with no bounds check. The crypto core
> > > > does not enforce slen <= seedsize before calling into the driver, so a
> > > > userspace caller via AF_ALG setsockopt(ALG_SET_KEY) can pass up to
> > > > sysctl_optmem_max bytes, overflowing the fixed-size buffer and
> > > > corrupting adjacent heap memory.
> > > >
> > > > Clamp the copy length to the buffer size, matching the approach used by
> > > > loongson-rng for oversized seeds.
> > > >
> > > > Discovered by Atuin - Automated Vulnerability Discovery Engine.
> > > >
> > > > Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
> > > > Cc: stable@vger.kernel.org
> > > > Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
> > > > ---
> > > > v2: Silently clamp oversized seeds with min_t instead of returning
> > > > -EINVAL (Herbert Xu).
> > >
> > > sun4i-ss-prng.c is useless, is still broken, and should just be deleted.
> >
> > Hello
> >
> > useless ? clearly no, it helped a lot on devices where it is.
>
> The only way this code is reachable is via "rng" algorithm type in
> AF_ALG, which is almost never used. Everyone just uses the regular
> Linux RNG (/dev/random etc) instead, as they should.
>
> In fact, anyone were to accidentally use this it would be a security
> vulnerability, seeing as sun4i_ss_prng_generate() doesn't actually fill
> in all the bytes that were requested. It also doesn't wait for the FIFO
> to be ready when reading data from it.
>
> Is it possible that there's a misunderstanding here and you think this
> provides entropy to the regular Linux RNG? It doesn't. hwrng does
> that, crypto_rng does not.
>
I believe to have used rngd for that. ( or something like that)
Anyway, I understand why want to remove it.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v2] crypto: sun4i-ss - clamp PRNG seed length to prevent heap overflow
2026-06-01 14:57 ` Corentin Labbe
@ 2026-06-01 15:47 ` Eric Biggers
0 siblings, 0 replies; 13+ messages in thread
From: Eric Biggers @ 2026-06-01 15:47 UTC (permalink / raw)
To: Corentin Labbe
Cc: Tianchu Chen, herbert, davem, wens, jernej.skrabec, samuel,
linux-crypto, linux-arm-kernel, linux-sunxi, linux-kernel
On Mon, Jun 01, 2026 at 04:57:21PM +0200, Corentin Labbe wrote:
> Le Fri, May 29, 2026 at 05:33:41PM +0000, Eric Biggers a écrit :
> > On Fri, May 29, 2026 at 07:10:06PM +0200, Corentin Labbe wrote:
> > > Le Fri, May 29, 2026 at 09:10:57AM -0700, Eric Biggers a écrit :
> > > > On Fri, May 29, 2026 at 08:08:01AM +0000, Tianchu Chen wrote:
> > > > > From: Tianchu Chen <flynnnchen@tencent.com>
> > > > >
> > > > > sun4i_ss_prng_seed() copies the user-supplied seed into ss->seed
> > > > > using the user-provided length with no bounds check. The crypto core
> > > > > does not enforce slen <= seedsize before calling into the driver, so a
> > > > > userspace caller via AF_ALG setsockopt(ALG_SET_KEY) can pass up to
> > > > > sysctl_optmem_max bytes, overflowing the fixed-size buffer and
> > > > > corrupting adjacent heap memory.
> > > > >
> > > > > Clamp the copy length to the buffer size, matching the approach used by
> > > > > loongson-rng for oversized seeds.
> > > > >
> > > > > Discovered by Atuin - Automated Vulnerability Discovery Engine.
> > > > >
> > > > > Fixes: 6298e948215f ("crypto: sunxi-ss - Add Allwinner Security System crypto accelerator")
> > > > > Cc: stable@vger.kernel.org
> > > > > Signed-off-by: Tianchu Chen <flynnnchen@tencent.com>
> > > > > ---
> > > > > v2: Silently clamp oversized seeds with min_t instead of returning
> > > > > -EINVAL (Herbert Xu).
> > > >
> > > > sun4i-ss-prng.c is useless, is still broken, and should just be deleted.
> > >
> > > Hello
> > >
> > > useless ? clearly no, it helped a lot on devices where it is.
> >
> > The only way this code is reachable is via "rng" algorithm type in
> > AF_ALG, which is almost never used. Everyone just uses the regular
> > Linux RNG (/dev/random etc) instead, as they should.
> >
> > In fact, anyone were to accidentally use this it would be a security
> > vulnerability, seeing as sun4i_ss_prng_generate() doesn't actually fill
> > in all the bytes that were requested. It also doesn't wait for the FIFO
> > to be ready when reading data from it.
> >
> > Is it possible that there's a misunderstanding here and you think this
> > provides entropy to the regular Linux RNG? It doesn't. hwrng does
> > that, crypto_rng does not.
> >
>
> I believe to have used rngd for that. ( or something like that)
>
> Anyway, I understand why want to remove it.
rngd doesn't use AF_ALG. So no, it couldn't have used this driver.
- Eric
^ permalink raw reply [flat|nested] 13+ messages in thread