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: <a3373ad5f92a4120bd0c8e0c751eb7617e035cf6.camel@xry111.site>
Date: Thu, 29 Aug 2024 22:06:47 +0800
From: Xi Ruoyao <xry111@...111.site>
To: "Jason A. Donenfeld" <Jason@...c4.com>, Tiezhu Yang
 <yangtiezhu@...ngson.cn>
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>, 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, 2024-08-29 at 15:37 +0200, Jason A. Donenfeld wrote:
> 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.

$ unshare -r -T --boottime $((365*24*3600))
# uptime
 21:54:36 up 365 days,  5:38,  0 user,  load average: 0.05, 0.08, 2.82
# /home/xry111/git-repos/linux/tools/testing/selftests/vDSO/vdso_test_getrandom bench-single
   vdso: 25000000 times in 0.499528591 seconds
   libc: 25000000 times in 6.968980040 seconds
syscall: 25000000 times in 6.987357071 seconds

So it seems normal in a time ns.

And from a comment in arch/loongarch/include/asm/vdso/vdso.h:

/*
 * The layout of vvar:
 *
 *                      high
 * +---------------------+--------------------------+
 * | loongarch vdso data | LOONGARCH_VDSO_DATA_SIZE |
 * +---------------------+--------------------------+
 * |  time-ns vdso data  |        PAGE_SIZE         |
 * +---------------------+--------------------------+
 * |  generic vdso data  |        PAGE_SIZE         |
 * +---------------------+--------------------------+
 *                      low
 */

And VVAR_LOONGARCH_PAGES_START is 2:

enum vvar_pages {
    VVAR_GENERIC_PAGE_OFFSET,
    VVAR_TIMENS_PAGE_OFFSET,
    VVAR_LOONGARCH_PAGES_START,
    VVAR_LOONGARCH_PAGES_END = VVAR_LOONGARCH_PAGES_START +
LOONGARCH_VDSO_DATA_PAGES - 1,
    VVAR_NR_PAGES,
};

So get_vdso_data() + VVAR_LOONGARCH_PAGES_START * PAGE_SIZE should have
already "jumped over" the time-ns vdso data.

OTOH it seems we are wasting a page if !CONFIG_TIME_NS.	Maybe:

enum vvar_pages {
    VVAR_GENERIC_PAGE_OFFSET,
#ifdef CONFIG_TIME_NS
    VVAR_TIMENS_PAGE_OFFSET,
#endif
    VVAR_LOONGARCH_PAGES_START,
    VVAR_LOONGARCH_PAGES_END = VVAR_LOONGARCH_PAGES_START +
LOONGARCH_VDSO_DATA_PAGES - 1,
    VVAR_NR_PAGES,
};

Tiezhu: how do you think?


-- 
Xi Ruoyao <xry111@...111.site>
School of Aerospace Science and Technology, Xidian University