linux-kernel - Re: [PATCH] aarch64: vdso: Wire up getrandom() vDSO implementation

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: <Zs2UGH6xjJmis5XD@zx2c4.com>
Date: Tue, 27 Aug 2024 10:53:44 +0200
From: "Jason A. Donenfeld" <Jason@...c4.com>
To: Christophe Leroy <christophe.leroy@...roup.eu>
Cc: Adhemerval Zanella <adhemerval.zanella@...aro.org>,
	Theodore Ts'o <tytso@....edu>, linux-kernel@...r.kernel.org,
	linux-crypto@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linux-arch@...r.kernel.org,
	Catalin Marinas <catalin.marinas@....com>,
	Will Deacon <will@...nel.org>, Thomas Gleixner <tglx@...utronix.de>,
	Eric Biggers <ebiggers@...nel.org>
Subject: Re: [PATCH] aarch64: vdso: Wire up getrandom() vDSO implementation

On Tue, Aug 27, 2024 at 10:46:21AM +0200, Christophe Leroy wrote:
> > +/**
> > + * __arch_chacha20_blocks_nostack - Generate ChaCha20 stream without using the stack.
> > + * @dst_bytes:	Destination buffer to hold @nblocks * 64 bytes of output.
> > + * @key:	32-byte input key.
> > + * @counter:	8-byte counter, read on input and updated on return.
> > + * @nblocks:	Number of blocks to generate.
> > + *
> > + * Generates a given positive number of blocks of ChaCha20 output with nonce=0, and does not write
> > + * to any stack or memory outside of the parameters passed to it, in order to mitigate stack data
> > + * leaking into forked child processes.
> > + */
> > +extern void __arch_chacha20_blocks_nostack(u8 *dst_bytes, const u32 *key, u32 *counter, size_t nblocks);
> 
> For Jason: We all redefine this prototype, should we have it in a 
> central place, or do you expect some architecture to provide some static 
> inline for it ?

Given the doc comment and such, that would be nice. But I didn't see a
straight forward way of doing that when I tried before. If you want to
try and send another fixup commit, that'd be welcomed.

> > +#define __VDSO_RND_DATA_OFFSET  480
> > +
> 
> How is this offset calculated or defined ? What happens if the other 
> structures grow ? Could you use some sizeof(something) instead of 
> something from asm-offsets if you also need it in ASM ?

FYI, there's a similar static calculation like this in the x86 code:

+#if !defined(_SINGLE_DATA)
+#define _SINGLE_DATA
+DECLARE_VVAR_SINGLE(640, struct vdso_rng_data, _vdso_rng_data)
+#endif

> >   uname_M := $(shell uname -m 2>/dev/null || echo not)
> > -ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/)
> > +ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/x86/ -e s/x86_64/x86/ -e s/aarch64.*/arm64/)
> 
> >   SODIUM := $(shell pkg-config --libs libsodium 2>/dev/null)
> >   
> >   TEST_GEN_PROGS := vdso_test_gettimeofday
> > @@ -11,7 +11,7 @@ ifeq ($(ARCH),$(filter $(ARCH),x86 x86_64))
> >   TEST_GEN_PROGS += vdso_standalone_test_x86
> >   endif
> >   TEST_GEN_PROGS += vdso_test_correctness
> > -ifeq ($(uname_M),x86_64)
> > +ifeq ($(uname_M), $(filter x86_64 aarch64, $(uname_M)))
> 
> Does that work for you when you cross-compile ? For powerpc when I cross 
> compile I still get the x86_64 from uname_M here, which is unexpected.

That sounds like a legitimate bug you're pointing out, but not one with
Adhemerval's code, right? Rather, it's something to be fixed inside of
these self tests as a whole?

Jason