linux-kernel - Re: [PATCH v5] LoongArch: 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: <ZtB5pqfp0Lg6lzz6@zx2c4.com>
Date: Thu, 29 Aug 2024 15:37:42 +0200
From: "Jason A. Donenfeld" <Jason@...c4.com>
To: Xi Ruoyao <xry111@...111.site>
Cc: Huacai Chen <chenhuacai@...nel.org>, WANG Xuerui <kernel@...0n.name>,
	linux-crypto@...r.kernel.org, loongarch@...ts.linux.dev,
	linux-kernel@...r.kernel.org, Jinyang He <hejinyang@...ngson.cn>,
	Tiezhu Yang <yangtiezhu@...ngson.cn>, Arnd Bergmann <arnd@...db.de>,
	Thomas Gleixner <tglx@...utronix.de>,
	Christophe Leroy <christophe.leroy@...roup.eu>
Subject: Re: [PATCH v5] LoongArch: vDSO: Wire up getrandom() vDSO
 implementation

On Thu, Aug 29, 2024 at 03:27:33PM +0200, Jason A. Donenfeld wrote:
> One small question just occurred to me:
> 
> > +static __always_inline const struct vdso_rng_data *__arch_get_vdso_rng_data(
> > +	void)
> > +{
> > +	return (const struct vdso_rng_data *)(
> > +		get_vdso_data() +
> > +		VVAR_LOONGARCH_PAGES_START * PAGE_SIZE +
> > +		offsetof(struct loongarch_vdso_data, rng_data));
> > +}
> 
> Did you test this in a TIMENS? On x86, I had to deal with the page
> offsets switching around depending on whether there was a TIMENS. I
> tested this in my test harness with some basic code like:
> 
>        if (argc == 1) {
>                if (unshare(CLONE_NEWTIME))
>                        panic("unshare(CLONE_NEWTIME)");
>                if (!fork()) {
>                        if (execl(argv[0], argv[0], "now-in-timens"))
>                                panic("execl");
>                }
>                wait(NULL);
>                poweroff();
>        }
> 
> Because unlike other namespaces, the time one only becomes active after
> fork/exec.
> 
> But maybe loongarch is more organized and you don't need any special
> handling in __arch_get_vdso...data() functions like I needed on x86.
> Just thought I should check.

Normal results:

   vdso: 25000000 times in 0.287330836 seconds
   libc: 25000000 times in 4.480710835 seconds
syscall: 25000000 times in 4.411098048 seconds

After applying

diff --git a/arch/x86/include/asm/vdso/getrandom.h b/arch/x86/include/asm/vdso/getrandom.h
index ff5334ad32a0..5cb1b318ebe3 100644
--- a/arch/x86/include/asm/vdso/getrandom.h
+++ b/arch/x86/include/asm/vdso/getrandom.h
@@ -32,8 +32,6 @@ static __always_inline ssize_t getrandom_syscall(void *buffer, size_t len, unsig

 static __always_inline const struct vdso_rng_data *__arch_get_vdso_rng_data(void)
 {
-	if (IS_ENABLED(CONFIG_TIME_NS) && __vdso_data->clock_mode == VDSO_CLOCKMODE_TIMENS)
-		return (void *)&__vdso_rng_data + ((void *)&__timens_vdso_data - (void *)&__vdso_data);
 	return &__vdso_rng_data;
 }

the results are:

   vdso: 25000000 times in 4.403789593 seconds
   libc: 25000000 times in 4.466771093 seconds
syscall: 25000000 times in 4.428145416 seconds

The difference is that when it finds the shared data in the wrong place,
it thinks the RNG is uninitialized, so it always falls back to the
syscall, hence all three times being the same.

If you're unsure how timens handling works on loongarch, try this test
yourself and see what you get.

Jason