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...
