| Message ID | af749a8447bd7f0e9dd26ca6c87e9c6afecb09d9@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-sunxi+bounces-23691-sunxi=pue.re@lists.linux.dev> X-Original-To: noreply@patchwork.local Delivered-To: noreply@patchwork.local Received: from sin.lore.kernel.org (sin.lore.kernel.org [104.64.211.4]) by mxe881.netcup.net (Postfix) with ESMTPS id B7FF41C025D for <noreply@patchwork.local>; Thu, 28 May 2026 17:11:00 +0200 (CEST) Authentication-Results: mxe881; dkim=pass header.d=linux.dev; spf=pass (sender IP is 104.64.211.4) smtp.mailfrom=linux-sunxi+bounces-23691-noreply=patchwork.local@lists.linux.dev smtp.helo=sin.lore.kernel.org Received-SPF: pass (mxe881: domain of lists.linux.dev designates 104.64.211.4 as permitted sender) client-ip=104.64.211.4; envelope-from=linux-sunxi+bounces-23691-noreply=patchwork.local@lists.linux.dev; helo=sin.lore.kernel.org; Received: from smtp.subspace.kernel.org (conduit.subspace.kernel.org [100.90.174.1]) by sin.lore.kernel.org (Postfix) with ESMTP id 309573053C92 for <noreply@patchwork.local>; Thu, 28 May 2026 15:00:27 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 843612E7363 for <noreply@patchwork.local>; Thu, 28 May 2026 15:00:26 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="njfeg0aU" X-Original-To: linux-sunxi@lists.linux.dev Received: from out-181.mta0.migadu.com (out-181.mta0.migadu.com [91.218.175.181]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 7BED545349A for <linux-sunxi@lists.linux.dev>; Thu, 28 May 2026 14:53:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.181 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779980010; cv=none; b=bCrFXbNFdtsdY2UrmuDBv74jognl0bizDdMzcWyhxIKXgpMCk5klkUnkmVZ/UDk8BDK6VYz1P7YEip6/zd7jqaLHFdwY11Kx5nQmZQxRORAYSvu1BJpHxHlEGb8hCk6BnjbM6NIX3YhMVNdLOYic3RJtjFZ1ldG4UGyhZutnIuc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779980010; c=relaxed/simple; bh=Ixh0YbBJf+pswoH+BffSh+FjEQOZ7B9JPVWmf9gtkao=; h=MIME-Version:Date:Content-Type:From:Message-ID:Subject:To:Cc; b=XgOWTiwkoviI9jYptZx9ZoKrSlzzFLCCgab1HKnCIDZO0loknbl2el6sJG+vAPUoxQSUDlZSEh+RTVWDxZnnPDSIIWnyU/NrEhmjyZdBIc3Otd/2uzoIkPNC5EQkT7k8lZbiYMlRM+68GhF6noDT+dHSYzXTSdNGY2c0GVxhwng= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=njfeg0aU; arc=none smtp.client-ip=91.218.175.181 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Precedence: bulk X-Mailing-List: linux-sunxi@lists.linux.dev List-Id: <linux-sunxi.lists.linux.dev> List-Subscribe: <mailto:linux-sunxi+subscribe@lists.linux.dev> List-Unsubscribe: <mailto:linux-sunxi+unsubscribe@lists.linux.dev> MIME-Version: 1.0 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1779980000; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=BMaA7h5kGMRU9hf/QRuYJW/W07yAJ1I7bh7FabXITYU=; b=njfeg0aU996w3zpMP+2q1ucUGMIvYDZSs21CeNIOfUiL1ZrI0VSfYke6OkZz2zMRnjkD9j OzS4sgXyyDAn5IBflYCdRtLbZwtsqaBKdQVzjCLPth+ha49m+0zG+Sdj6psnpFjZ5esy00 yLfOy1+rIG/JFMKekOtTFrmiKmtpp1U= Date: Thu, 28 May 2026 14:53:17 +0000 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: "Tianchu Chen" <tianchu.chen@linux.dev> Message-ID: <af749a8447bd7f0e9dd26ca6c87e9c6afecb09d9@linux.dev> TLS-Required: No Subject: [PATCH] crypto: sun4i-ss: restrict PRNG seed length to prevent heap overflow To: clabbe.montjoie@gmail.com, herbert@gondor.apana.org.au, davem@davemloft.net Cc: wens@kernel.org, jernej.skrabec@gmail.com, samuel@sholland.org, linux-crypto@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-kernel@vger.kernel.org X-Migadu-Flow: FLOW_OUT X-MORS-Enabled: yes X-MORS-DOMAIN: patchwork.local X-MORS-HOSTING: hosting172546 X-MORS-USER: hosting172546 X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= |
| Series |
crypto: sun4i-ss: restrict PRNG seed length to prevent heap overflow
|
|
Commit Message
Tianchu Chen
May 28, 2026, 2:53 p.m. UTC
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. Add a length check rejecting seeds larger than the buffer. 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> --- drivers/crypto/allwinner/sun4i-ss/sun4i-ss-prng.c | 2 ++ 1 file changed, 2 insertions(+)
Comments
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.
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.
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,
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
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; memcpy(algt->ss->seed, seed, slen); return 0;