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: <CAHmME9p+-0S-a3kjsv75irmLdGpW6rUWBVm87E4-Z9hpWyA7YA@mail.gmail.com>
Date: Thu, 29 Aug 2024 16:44:36 +0200
From: "Jason A. Donenfeld" <Jason@...c4.com>
To: Xi Ruoyao <xry111@...111.site>
Cc: Tiezhu Yang <yangtiezhu@...ngson.cn>, 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, Aug 29, 2024 at 4:06 PM Xi Ruoyao <xry111@...111.site> wrote:
>
> 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.

Oh good. Thanks for checking. So it sounds like there's just Huacai's
set of comments and we're good.