| Message ID | 4d4407c05835a50413fa1e974e3aa3f4abfe2d5b@linux.dev (mailing list archive) |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-sunxi+bounces-23696-sunxi=pue.re@lists.linux.dev> X-Original-To: noreply@patchwork.local Delivered-To: noreply@patchwork.local Received: from tor.lore.kernel.org (tor.lore.kernel.org [172.105.105.114]) by mxe881.netcup.net (Postfix) with ESMTPS id DAC501C08B5 for <noreply@patchwork.local>; Fri, 29 May 2026 10:14:00 +0200 (CEST) Authentication-Results: mxe881; dkim=pass header.d=linux.dev; spf=pass (sender IP is 172.105.105.114) smtp.mailfrom=linux-sunxi+bounces-23696-noreply=patchwork.local@lists.linux.dev smtp.helo=tor.lore.kernel.org Received-SPF: pass (mxe881: domain of lists.linux.dev designates 172.105.105.114 as permitted sender) client-ip=172.105.105.114; envelope-from=linux-sunxi+bounces-23696-noreply=patchwork.local@lists.linux.dev; helo=tor.lore.kernel.org; Received: from smtp.subspace.kernel.org (conduit.subspace.kernel.org [100.90.174.1]) by tor.lore.kernel.org (Postfix) with ESMTP id D46D03004278 for <noreply@patchwork.local>; Fri, 29 May 2026 08:08:12 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id DC4382EC0B0; Fri, 29 May 2026 08:08:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="IzCxCgfY" X-Original-To: linux-sunxi@lists.linux.dev Received: from out-172.mta1.migadu.com (out-172.mta1.migadu.com [95.215.58.172]) (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 D0BD833F5B1 for <linux-sunxi@lists.linux.dev>; Fri, 29 May 2026 08:08:09 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=95.215.58.172 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780042091; cv=none; b=JapiS2X0TVgjh/pBmeT7ecrdT56QQ7h1sptV048gseMLrs0iJ9do/soMW1+HJcNcYbFLwlEo4PQWtcRR5zG0+nChKQkEJGVVmmWmaxhRhvbx0V1XdC2+bgpLZtxuf66OQEVQykHP4l+c5Mxy9qEkh0u+pFfzeFj1Ain/78l5+iQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1780042091; c=relaxed/simple; bh=pwbq1GQXPiFQwNmyslF6wkzZlnjwVzAbBnldA7Mxa1U=; h=MIME-Version:Date:Content-Type:From:Message-ID:Subject:To:Cc: In-Reply-To:References; b=fzJR2TJ72icy3gQkiWTlqPO2SjcFQCY3s1n5/R4WDUwxEnwf3UyjQyc/h5m257OkwtWetGo6nhre1tQQ6C8luJ1FdnsJEOGgjojKMeMw9i9OBq6ImXGtlNM93oeHvTe6AxHS1QljgQdOSwxgWr9lHbzZYVpWe6wZ8kU/Lu3TpmU= 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=IzCxCgfY; arc=none smtp.client-ip=95.215.58.172 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=1780042087; 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: in-reply-to:in-reply-to:references:references; bh=cn8RGbbWhMK+fmOfLvNDrpH9fQ9BHow0UZabfx8bzzE=; b=IzCxCgfYrXXqAra8csjkLDYzBUaXEP1y+20GkUNHakvuHgvPcnDcVazIECq/qCx+KlXa51 z9zMh+/V02p5KPJoKO5ii3Pkefr2lloZdWh0UE1xSxUpDsPzd/rR0+qFVIIf51SCtyi4Aw tnYQobxdi72BP9jrI96qSccPDLP3woI= Date: Fri, 29 May 2026 08:08:01 +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: <4d4407c05835a50413fa1e974e3aa3f4abfe2d5b@linux.dev> TLS-Required: No Subject: [PATCH v2] crypto: sun4i-ss - clamp 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 In-Reply-To: <af749a8447bd7f0e9dd26ca6c87e9c6afecb09d9@linux.dev> References: <af749a8447bd7f0e9dd26ca6c87e9c6afecb09d9@linux.dev> 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 |
[v2] crypto: sun4i-ss - clamp PRNG seed length to prevent heap overflow
|
|
Commit Message
Tianchu Chen
May 29, 2026, 8:08 a.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. 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(-)
Comments
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?
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
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
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
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
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; }