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

[Adhemerval Zanella Netto <adhemerval.zanella@linaro.org>]

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.