| 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
| ||
|
Message-ID: <7d9c376c-10ae-40cc-8d8a-d614b8dd289f@linaro.org>
Date: Tue, 27 Aug 2024 11:35:26 -0300
From: Adhemerval Zanella Netto <adhemerval.zanella@...aro.org>
To: "Jason A. Donenfeld" <Jason@...c4.com>
Cc: Christophe Leroy <christophe.leroy@...roup.eu>,
Theodore Ts'o <tytso@....edu>, linux-kernel@...r.kernel.org,
linux-crypto@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-arch@...r.kernel.org, Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>, Thomas Gleixner <tglx@...utronix.de>,
Eric Biggers <ebiggers@...nel.org>
Subject: Re: [PATCH] aarch64: vdso: Wire up getrandom() vDSO implementation
On 27/08/24 11:32, Jason A. Donenfeld wrote:
> On Tue, Aug 27, 2024 at 4:30 PM Adhemerval Zanella Netto
> <adhemerval.zanella@...aro.org> 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@...aro.org. 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@...aro.org. 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...
Oops, it does work indeed (my mistake here).
Powered by blists - more mailing lists