linux-kernel - Re: [RFC/RFT PATCH 5/6] random: Plug race in preceding patch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20251128111328.68354182@pumpkin>
Date: Fri, 28 Nov 2025 11:13:28 +0000
From: david laight <david.laight@...box.com>
To: Ard Biesheuvel <ardb+git@...gle.com>
Cc: linux-hardening@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 linux-kernel@...r.kernel.org, Ard Biesheuvel <ardb@...nel.org>, Kees Cook
 <kees@...nel.org>, Ryan Roberts <ryan.roberts@....com>, Will Deacon
 <will@...nel.org>, Arnd Bergmann <arnd@...db.de>, Jeremy Linton
 <jeremy.linton@....com>, Catalin Marinas <Catalin.Marinas@....com>, Mark
 Rutland <mark.rutland@....com>, "Jason A. Donenfeld" <Jason@...c4.com>
Subject: Re: [RFC/RFT PATCH 5/6] random: Plug race in preceding patch

On Thu, 27 Nov 2025 10:22:32 +0100
Ard Biesheuvel <ardb+git@...gle.com> wrote:

> From: Ard Biesheuvel <ardb@...nel.org>
> 
> The lockless get_random_uXX() reads the next value from the linear
> buffer and then overwrites it with a 0x0 value. This is racy, as the
> code might be re-entered by an interrupt handler, and so the store might
> redundantly wipe the location accessed by the interrupt context rather
> than the interrupted context.

Is overwriting the used value even useful?
If someone manages to read the 'last' value, then they can equally read
the 'next' one - which is likely to be just as useful.
Moreover the zeros tell anyone who has managed the access the buffer
which entry will be used next - without having to find 'batch->position'.

There is probably more to gain from putting the control data in a
completely different piece of memory from the buffer.

	David

> 
> To plug this race, wipe the preceding location when reading the next
> value from the linear buffer. Given that the position is always non-zero
> outside of the critical section, this is guaranteed to be safe, and
> ensures that the produced values are always wiped from the buffer.
> 
> Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
> ---
>  drivers/char/random.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/char/random.c b/drivers/char/random.c
> index 71bd74871540..e8ba460c5c9c 100644
> --- a/drivers/char/random.c
> +++ b/drivers/char/random.c
> @@ -547,6 +547,7 @@ type get_random_ ##type(void)							\
>  	next = (u64)next_gen << 32;						\
>  	if (likely(batch->position < ARRAY_SIZE(batch->entropy))) {		\
>  		next |=	batch->position + 1; /* next-1 is bogus otherwise */	\
> +		batch->entropy[batch->position - 1] = 0;			\
>  		ret = batch->entropy[batch->position];				\
>  	}									\
>  	if (cmpxchg64_local(&batch->posgen, next, next - 1) != next - 1) {	\