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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Aug 27, 2024 at 4:30 PM Adhemerval Zanella Netto
<adhemerval.zanella@xxxxxxxxxx> wrote:
>
>
>
> On 27/08/24 11:28, Jason A. Donenfeld wrote:
> > On Tue, Aug 27, 2024 at 11:14:27AM -0300, Adhemerval Zanella Netto wrote:
> >>
> >>
> >> On 27/08/24 11:10, Christophe Leroy wrote:
> >>>
> >>>
> >>> Le 27/08/2024 à 16:01, Adhemerval Zanella Netto a écrit :
> >>>> [Vous ne recevez pas souvent de courriers de adhemerval.zanella@xxxxxxxxxx. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> >>>>
> >>>> On 27/08/24 11:00, Christophe Leroy wrote:
> >>>>>
> >>>>>
> >>>>> Le 27/08/2024 à 15:39, Adhemerval Zanella Netto a écrit :
> >>>>>> [Vous ne recevez pas souvent de courriers de adhemerval.zanella@xxxxxxxxxx. Découvrez pourquoi ceci est important à https://aka.ms/LearnAboutSenderIdentification ]
> >>>>>>
> >>>>>> On 27/08/24 10:34, Jason A. Donenfeld wrote:
> >>>>>>> On Tue, Aug 27, 2024 at 10:17:18AM -0300, Adhemerval Zanella Netto wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 26/08/24 17:27, Jason A. Donenfeld wrote:
> >>>>>>>>> Hi Adhemerval,
> >>>>>>>>>
> >>>>>>>>> Thanks for posting this! Exciting to have it here.
> >>>>>>>>>
> >>>>>>>>> Just some small nits for now:
> >>>>>>>>>
> >>>>>>>>> On Mon, Aug 26, 2024 at 06:10:40PM +0000, Adhemerval Zanella wrote:
> >>>>>>>>>> +static __always_inline ssize_t getrandom_syscall(void *buffer, size_t len, unsigned int flags)
> >>>>>>>>>> +{
> >>>>>>>>>> +  register long int x8 asm ("x8") = __NR_getrandom;
> >>>>>>>>>> +  register long int x0 asm ("x0") = (long int) buffer;
> >>>>>>>>>> +  register long int x1 asm ("x1") = (long int) len;
> >>>>>>>>>> +  register long int x2 asm ("x2") = (long int) flags;
> >>>>>>>>>
> >>>>>>>>> Usually it's written just as `long` or `unsigned long`, and likewise
> >>>>>>>>> with the cast. Also, no space after the cast.
> >>>>>>>>
> >>>>>>>> Ack.
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> +#define __VDSO_RND_DATA_OFFSET  480
> >>>>>>>>>
> >>>>>>>>> This is the size of the data currently there?
> >>>>>>>>
> >>>>>>>> Yes, I used the same strategy x86 did.
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>>>    #include <asm/page.h>
> >>>>>>>>>>    #include <asm/vdso.h>
> >>>>>>>>>>    #include <asm-generic/vmlinux.lds.h>
> >>>>>>>>>> +#include <vdso/datapage.h>
> >>>>>>>>>> +#include <asm/vdso/vsyscall.h>
> >>>>>>>>>
> >>>>>>>>> Possible to keep the asm/ together?
> >>>>>>>>
> >>>>>>>> Ack.
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> + * ARM64 ChaCha20 implementation meant for vDSO.  Produces a given positive
> >>>>>>>>>> + * number of blocks of output with nonnce 0, taking an input key and 8-bytes
> >>>>>>>>>
> >>>>>>>>> nonnce -> nonce
> >>>>>>>>
> >>>>>>>> Ack.
> >>>>>>>>
> >>>>>>>>>
> >>>>>>>>>> -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)))
> >>>>>>>>>>    TEST_GEN_PROGS += vdso_test_getrandom
> >>>>>>>>>>    ifneq ($(SODIUM),)
> >>>>>>>>>>    TEST_GEN_PROGS += vdso_test_chacha
> >>>>>>>>>
> >>>>>>>>> You'll need to add the symlink to get the chacha selftest running:
> >>>>>>>>>
> >>>>>>>>>     $ ln -s ../../../arch/arm64/kernel/vdso tools/arch/arm64/vdso
> >>>>>>>>>     $ git add tools/arch/arm64/vdso
> >>>>>>>>>
> >>>>>>>>> Also, can you confirm that the chacha selftest runs and works?
> >>>>>>>>
> >>>>>>>> Yes, last time I has to built it manually since the Makefile machinery seem
> >>>>>>>> to be broken even on x86_64.  In a Ubuntu vm I have:
> >>>>>>>>
> >>>>>>>> tools/testing/selftests/vDSO$ make
> >>>>>>>>     CC       vdso_test_gettimeofday
> >>>>>>>>     CC       vdso_test_getcpu
> >>>>>>>>     CC       vdso_test_abi
> >>>>>>>>     CC       vdso_test_clock_getres
> >>>>>>>>     CC       vdso_standalone_test_x86
> >>>>>>>>     CC       vdso_test_correctness
> >>>>>>>>     CC       vdso_test_getrandom
> >>>>>>>>     CC       vdso_test_chacha
> >>>>>>>> In file included from /home/azanella/Projects/linux/linux-git/include/linux/limits.h:7,
> >>>>>>>>                    from /usr/include/x86_64-linux-gnu/bits/local_lim.h:38,
> >>>>>>>>                    from /usr/include/x86_64-linux-gnu/bits/posix1_lim.h:161,
> >>>>>>>>                    from /usr/include/limits.h:195,
> >>>>>>>>                    from /usr/lib/gcc/x86_64-linux-gnu/13/include/limits.h:205,
> >>>>>>>>                    from /usr/lib/gcc/x86_64-linux-gnu/13/include/syslimits.h:7,
> >>>>>>>>                    from /usr/lib/gcc/x86_64-linux-gnu/13/include/limits.h:34,
> >>>>>>>>                    from /usr/include/sodium/export.h:7,
> >>>>>>>>                    from /usr/include/sodium/crypto_stream_chacha20.h:14,
> >>>>>>>>                    from vdso_test_chacha.c:6:
> >>>>>>>> /usr/include/x86_64-linux-gnu/bits/xopen_lim.h:99:6: error: missing binary operator before token "("
> >>>>>>>>      99 | # if INT_MAX == 32767
> >>>>>>>>         |      ^~~~~~~
> >>>>>>>> /usr/include/x86_64-linux-gnu/bits/xopen_lim.h:102:7: error: missing binary operator before token "("
> >>>>>>>>     102 | #  if INT_MAX == 2147483647
> >>>>>>>>         |       ^~~~~~~
> >>>>>>>> /usr/include/x86_64-linux-gnu/bits/xopen_lim.h:126:6: error: missing binary operator before token "("
> >>>>>>>>     126 | # if LONG_MAX == 2147483647
> >>>>>>>>         |      ^~~~~~~~
> >>>>>>>> make: *** [../lib.mk:222: /home/azanella/Projects/linux/linux-git/tools/testing/selftests/vDSO/vdso_test_chacha] Error 1
> >>>>>>>
> >>>>>>> You get that even with the latest random.git? I thought Christophe's
> >>>>>>> patch fixed that, but maybe not and I should just remove the dependency
> >>>>>>> on the sodium header instead.
> >>>>>>
> >>>>>> On x86_64 I tested with Linux master.  With random.git it is a different issue:
> >>>>>>
> >>>>>> linux-git/tools/testing/selftests/vDSO$ make
> >>>>>>     CC       vdso_test_gettimeofday
> >>>>>>     CC       vdso_test_getcpu
> >>>>>>     CC       vdso_test_abi
> >>>>>>     CC       vdso_test_clock_getres
> >>>>>>     CC       vdso_standalone_test_x86
> >>>>>>     CC       vdso_test_correctness
> >>>>>>     CC       vdso_test_getrandom
> >>>>>>     CC       vdso_test_chacha
> >>>>>> /usr/bin/ld: /tmp/ccKpjnSM.o: in function `main':
> >>>>>> vdso_test_chacha.c:(.text+0x276): undefined reference to `crypto_stream_chacha20'
> >>>>>> collect2: error: ld returned 1 exit status
> >>>>>>
> >>>>>> If I move -lsodium to the end of the compiler command it works.
> >>>>>>
> >>>>>>
> >>>>>
> >>>>> Try a "make clean" maybe ?
> >>>>>
> >>>>> I have Fedora 38 and no build problem with latest random tree:
> >>>>>
> >>>>> $ make V=1
> >>>>> gcc -std=gnu99 -D_GNU_SOURCE=    vdso_test_gettimeofday.c parse_vdso.c -o /home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_gettimeofday
> >>>>> gcc -std=gnu99 -D_GNU_SOURCE=    vdso_test_getcpu.c parse_vdso.c  -o /home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_getcpu
> >>>>> gcc -std=gnu99 -D_GNU_SOURCE=    vdso_test_abi.c parse_vdso.c  -o /home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_abi
> >>>>> gcc -std=gnu99 -D_GNU_SOURCE=    vdso_test_clock_getres.c  -o /home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_clock_getres
> >>>>> gcc -std=gnu99 -D_GNU_SOURCE= -nostdlib -fno-asynchronous-unwind-tables -fno-stack-protector    vdso_standalone_test_x86.c parse_vdso.c  -o /home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_standalone_test_x86
> >>>>> gcc -std=gnu99 -D_GNU_SOURCE=  -ldl  vdso_test_correctness.c  -o /home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_correctness
> >>>>> gcc -std=gnu99 -D_GNU_SOURCE= -isystem /home/chleroy/linux-powerpc/tools/testing/selftests/../../../tools/include -isystem /home/chleroy/linux-powerpc/tools/testing/selftests/../../../include/uapi    vdso_test_getrandom.c parse_vdso.c  -o /home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_getrandom
> >>>>> gcc -std=gnu99 -D_GNU_SOURCE= -idirafter /home/chleroy/linux-powerpc/tools/testing/selftests/../../../tools/include -idirafter /home/chleroy/linux-powerpc/tools/testing/selftests/../../../arch/x86/include -idirafter /home/chleroy/linux-powerpc/tools/testing/selftests/../../../include -D__ASSEMBLY__ -DBULID_VDSO -DCONFIG_FUNCTION_ALIGNMENT=0 -Wa,--noexecstack -lsodium     vdso_test_chacha.c /home/chleroy/linux-powerpc/tools/testing/selftests/../../../tools/arch/x86/vdso/vgetrandom-chacha.S  -o /home/chleroy/linux-powerpc/tools/testing/selftests/vDSO/vdso_test_chacha
> >>>>> $
> >>>>
> >>>> It is a clean tree (git clean -dfx), and I take there is no need to build a kernel
> >>>> prior hand.
> >>>
> >>> I meeant 'make clean'
> >>>
> >>>
> >>> Right, I have not built any x86 kernel at the moment.
> >>>
> >>> Just :
> >>> $ pwd
> >>> /home/chleroy/linux-powerpc/tools/testing/selftests/vDSO
> >>>
> >>> $ make clean
> >>>
> >>> then
> >>>
> >>> $ make V=1
> >>
> >> The issue is Ubuntu linker is configure to use --as-needed by default, this
> >> patch fixes the issue:
> >>
> >> diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
> >> index 10ffdda3f2fa..151baf650e4c 100644
> >> --- a/tools/testing/selftests/vDSO/Makefile
> >> +++ b/tools/testing/selftests/vDSO/Makefile
> >> @@ -45,4 +45,4 @@ $(OUTPUT)/vdso_test_chacha: CFLAGS += -idirafter $(top_srcdir)/tools/include \
> >>                                        -idirafter $(top_srcdir)/arch/$(ARCH)/include \
> >>                                        -idirafter $(top_srcdir)/include \
> >>                                        -D__ASSEMBLY__ -DBULID_VDSO -DCONFIG_FUNCTION_ALIGNMENT=0 \
> >> -                                      -Wa,--noexecstack $(SODIUM)
> >> +                                      -Wa,--noexecstack -Wl,-no-as-needed $(SODIUM)
> >
> > Oh, it's an as-needed thing. In that case, does this fix it for you?
> >
> > diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
> > index 10ffdda3f2fa..834aa862ba2c 100644
> > --- a/tools/testing/selftests/vDSO/Makefile
> > +++ b/tools/testing/selftests/vDSO/Makefile
> > @@ -1,7 +1,8 @@
> >  # SPDX-License-Identifier: GPL-2.0
> >  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/)
> > -SODIUM := $(shell pkg-config --libs libsodium 2>/dev/null)
> > +SODIUM_LIBS := $(shell pkg-config --libs libsodium 2>/dev/null)
> > +SODIUM_CFLAGS := $(shell pkg-config --cflags libsodium 2>/dev/null)
> >
> >  TEST_GEN_PROGS := vdso_test_gettimeofday
> >  TEST_GEN_PROGS += vdso_test_getcpu
> > @@ -13,7 +14,7 @@ endif
> >  TEST_GEN_PROGS += vdso_test_correctness
> >  ifeq ($(uname_M),x86_64)
> >  TEST_GEN_PROGS += vdso_test_getrandom
> > -ifneq ($(SODIUM),)
> > +ifneq ($(SODIUM_LIBS),)
> >  TEST_GEN_PROGS += vdso_test_chacha
> >  endif
> >  endif
> > @@ -41,8 +42,9 @@ $(OUTPUT)/vdso_test_getrandom: CFLAGS += -isystem $(top_srcdir)/tools/include \
> >                                           -isystem $(top_srcdir)/include/uapi
> >
> >  $(OUTPUT)/vdso_test_chacha: $(top_srcdir)/tools/arch/$(ARCH)/vdso/vgetrandom-chacha.S
> > +$(OUTPUT)/vdso_test_chacha: LDLIBS += $(SODIUM_LIBS)
> >  $(OUTPUT)/vdso_test_chacha: CFLAGS += -idirafter $(top_srcdir)/tools/include \
> >                                        -idirafter $(top_srcdir)/arch/$(ARCH)/include \
> >                                        -idirafter $(top_srcdir)/include \
> >                                        -D__ASSEMBLY__ -DBULID_VDSO -DCONFIG_FUNCTION_ALIGNMENT=0 \
> > -                                      -Wa,--noexecstack $(SODIUM)
> > +                                      -Wa,--noexecstack $(SODIUM_CFLAGS)
> >
>
> Nops, 'pkg-config --cflags libsodium' is empty. The -Wl,-no-as-needed is simpler
> I think.

The --cflags thing is for a different issue Ruoyao found. My intended
fix here was the LDLIBS += $(SODIUM_LIBS) part, which moves the
`-lsodium` closer to the end of the command line. But it still doesn't
work? Surprising...





[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux