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: <397f9865-c4ad-44be-91ab-9764fe3aeb89@csgroup.eu>
Date: Tue, 27 Aug 2024 10:46:21 +0200
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: Adhemerval Zanella <adhemerval.zanella@...aro.org>,
 "Jason A . Donenfeld" <Jason@...c4.com>, 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



Le 26/08/2024 à 20:10, Adhemerval Zanella a écrit :
> Hook up the generic vDSO implementation to the aarch64 vDSO data page.
> The _vdso_rng_data required data is placed within the _vdso_data vvar
> page, by using a offset larger than the vdso_data
> (__VDSO_RND_DATA_OFFSET).
> 
> The vDSO function requires a ChaCha20 implementation that does not
> write to the stack, and that can do an entire ChaCha20 permutation.
> The one provided is based on the current chacha-neon-core.S and uses NEON
> on the permute operation.
> 
> Signed-off-by: Adhemerval Zanella <adhemerval.zanella@...aro.org>
> ---
>   arch/arm64/Kconfig                         |   1 +
>   arch/arm64/include/asm/vdso/getrandom.h    |  50 +++++++
>   arch/arm64/include/asm/vdso/vsyscall.h     |   9 ++
>   arch/arm64/kernel/vdso/Makefile            |   7 +-
>   arch/arm64/kernel/vdso/vdso.lds.S          |   4 +
>   arch/arm64/kernel/vdso/vgetrandom-chacha.S | 153 +++++++++++++++++++++
>   arch/arm64/kernel/vdso/vgetrandom.c        |  13 ++
>   tools/testing/selftests/vDSO/Makefile      |   4 +-
>   8 files changed, 238 insertions(+), 3 deletions(-)
>   create mode 100644 arch/arm64/include/asm/vdso/getrandom.h
>   create mode 100644 arch/arm64/kernel/vdso/vgetrandom-chacha.S
>   create mode 100644 arch/arm64/kernel/vdso/vgetrandom.c

Were you able to use selftests ? I think you are missing the symbolic 
link to vdso directory (assuming you are using latest master branch from 
https://git.kernel.org/pub/scm/linux/kernel/git/crng/random.git)

> 
> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
> index b3fc891f1544..e3f4c5bf0661 100644
> --- a/arch/arm64/Kconfig
> +++ b/arch/arm64/Kconfig
> @@ -237,6 +237,7 @@ config ARM64
>   	select HAVE_KPROBES
>   	select HAVE_KRETPROBES
>   	select HAVE_GENERIC_VDSO
> +	select VDSO_GETRANDOM

You don't keep things in alphabetical here order on ARM64 ?

>   	select HOTPLUG_CORE_SYNC_DEAD if HOTPLUG_CPU
>   	select IRQ_DOMAIN
>   	select IRQ_FORCED_THREADING
> diff --git a/arch/arm64/include/asm/vdso/getrandom.h b/arch/arm64/include/asm/vdso/getrandom.h
> new file mode 100644
> index 000000000000..6e2b136813ca
> --- /dev/null
> +++ b/arch/arm64/include/asm/vdso/getrandom.h
> @@ -0,0 +1,50 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +#ifndef __ASM_VDSO_GETRANDOM_H
> +#define __ASM_VDSO_GETRANDOM_H
> +
> +#ifndef __ASSEMBLY__
> +
> +#include <asm/unistd.h>
> +#include <vdso/datapage.h>
> +
> +/**
> + * getrandom_syscall - Invoke the getrandom() syscall.
> + * @buffer:	Destination buffer to fill with random bytes.
> + * @len:	Size of @buffer in bytes.
> + * @flags:	Zero or more GRND_* flags.
> + * Returns:	The number of random bytes written to @buffer, or a negative value indicating an error.
> + */
> +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;
> +
> +	asm ("svc 0" : "=r"(x0) : "r"(x8), "0"(x0), "r"(x1), "r"(x2));
> +
> +	return x0;
> +}
> +
> +static __always_inline const struct vdso_rng_data *__arch_get_vdso_rng_data(void)
> +{
> +	return &_vdso_rng_data;
> +}
> +
> +/**
> + * __arch_chacha20_blocks_nostack - Generate ChaCha20 stream without using the stack.
> + * @dst_bytes:	Destination buffer to hold @nblocks * 64 bytes of output.
> + * @key:	32-byte input key.
> + * @counter:	8-byte counter, read on input and updated on return.
> + * @nblocks:	Number of blocks to generate.
> + *
> + * Generates a given positive number of blocks of ChaCha20 output with nonce=0, and does not write
> + * to any stack or memory outside of the parameters passed to it, in order to mitigate stack data
> + * leaking into forked child processes.
> + */
> +extern void __arch_chacha20_blocks_nostack(u8 *dst_bytes, const u32 *key, u32 *counter, size_t nblocks);

For Jason: We all redefine this prototype, should we have it in a 
central place, or do you expect some architecture to provide some static 
inline for it ?

> +
> +#endif /* !__ASSEMBLY__ */
> +
> +#endif /* __ASM_VDSO_GETRANDOM_H */
> diff --git a/arch/arm64/include/asm/vdso/vsyscall.h b/arch/arm64/include/asm/vdso/vsyscall.h
> index f94b1457c117..7ddb2bc3b57b 100644
> --- a/arch/arm64/include/asm/vdso/vsyscall.h
> +++ b/arch/arm64/include/asm/vdso/vsyscall.h
> @@ -2,6 +2,8 @@
>   #ifndef __ASM_VDSO_VSYSCALL_H
>   #define __ASM_VDSO_VSYSCALL_H
>   
> +#define __VDSO_RND_DATA_OFFSET  480
> +

How is this offset calculated or defined ? What happens if the other 
structures grow ? Could you use some sizeof(something) instead of 
something from asm-offsets if you also need it in ASM ?

>   #ifndef __ASSEMBLY__
>   
>   #include <linux/timekeeper_internal.h>
> @@ -21,6 +23,13 @@ struct vdso_data *__arm64_get_k_vdso_data(void)
>   }
>   #define __arch_get_k_vdso_data __arm64_get_k_vdso_data
>   
> +static __always_inline
> +struct vdso_rng_data *__arm64_get_k_vdso_rnd_data(void)
> +{
> +	return (void *)__arm64_get_k_vdso_data() + __VDSO_RND_DATA_OFFSET;
> +}
> +#define __arch_get_k_vdso_rng_data __arm64_get_k_vdso_rnd_data
> +
>   static __always_inline
>   void __arm64_update_vsyscall(struct vdso_data *vdata, struct timekeeper *tk)
>   {
> diff --git a/arch/arm64/kernel/vdso/Makefile b/arch/arm64/kernel/vdso/Makefile
> index d11da6461278..37dad3bb953a 100644
> --- a/arch/arm64/kernel/vdso/Makefile
> +++ b/arch/arm64/kernel/vdso/Makefile
> @@ -9,7 +9,7 @@
>   # Include the generic Makefile to check the built vdso.
>   include $(srctree)/lib/vdso/Makefile
>   
> -obj-vdso := vgettimeofday.o note.o sigreturn.o
> +obj-vdso := vgettimeofday.o note.o sigreturn.o vgetrandom.o vgetrandom-chacha.o
>   
>   # Build rules
>   targets := $(obj-vdso) vdso.so vdso.so.dbg
> @@ -40,8 +40,13 @@ CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) -Os $(CC_FLAGS_SCS) \
>   				$(RANDSTRUCT_CFLAGS) $(GCC_PLUGINS_CFLAGS) \
>   				$(CC_FLAGS_LTO) $(CC_FLAGS_CFI) \
>   				-Wmissing-prototypes -Wmissing-declarations
> +CFLAGS_REMOVE_vgetrandom.o = $(CC_FLAGS_FTRACE) -Os $(CC_FLAGS_SCS) \
> +			     $(RANDSTRUCT_CFLAGS) $(GCC_PLUGINS_CFLAGS) \
> +			     $(CC_FLAGS_LTO) $(CC_FLAGS_CFI) \
> +			     -Wmissing-prototypes -Wmissing-declarations
>   
>   CFLAGS_vgettimeofday.o = -O2 -mcmodel=tiny -fasynchronous-unwind-tables
> +CFLAGS_vgetrandom.o = -O2 -mcmodel=tiny -fasynchronous-unwind-tables
>   
>   ifneq ($(c-gettimeofday-y),)
>     CFLAGS_vgettimeofday.o += -include $(c-gettimeofday-y)
> diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S
> index 45354f2ddf70..f8dbcece20e2 100644
> --- a/arch/arm64/kernel/vdso/vdso.lds.S
> +++ b/arch/arm64/kernel/vdso/vdso.lds.S
> @@ -12,6 +12,8 @@
>   #include <asm/page.h>
>   #include <asm/vdso.h>
>   #include <asm-generic/vmlinux.lds.h>
> +#include <vdso/datapage.h>
> +#include <asm/vdso/vsyscall.h>
>   
>   OUTPUT_FORMAT("elf64-littleaarch64", "elf64-bigaarch64", "elf64-littleaarch64")
>   OUTPUT_ARCH(aarch64)
> @@ -19,6 +21,7 @@ OUTPUT_ARCH(aarch64)
>   SECTIONS
>   {
>   	PROVIDE(_vdso_data = . - __VVAR_PAGES * PAGE_SIZE);
> +	PROVIDE(_vdso_rng_data = _vdso_data + __VDSO_RND_DATA_OFFSET);
>   #ifdef CONFIG_TIME_NS
>   	PROVIDE(_timens_data = _vdso_data + PAGE_SIZE);
>   #endif
> @@ -102,6 +105,7 @@ VERSION
>   		__kernel_gettimeofday;
>   		__kernel_clock_gettime;
>   		__kernel_clock_getres;
> +		__kernel_getrandom;
>   	local: *;
>   	};
>   }
> diff --git a/arch/arm64/kernel/vdso/vgetrandom-chacha.S b/arch/arm64/kernel/vdso/vgetrandom-chacha.S

[skipped ASM as I have not spoken ARM asm since I was at school in the 90's]

> diff --git a/arch/arm64/kernel/vdso/vgetrandom.c b/arch/arm64/kernel/vdso/vgetrandom.c
> new file mode 100644
> index 000000000000..b6d6f4db3a98
> --- /dev/null
> +++ b/arch/arm64/kernel/vdso/vgetrandom.c
> @@ -0,0 +1,13 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +
> +#include <linux/types.h>
> +#include <linux/mm.h>
> +
> +#include "../../../../lib/vdso/getrandom.c"

For gettimeofday ARM64 uses c-gettimeofday-y in the Makefile instead.

You should do the same with c-getrandom-y

> +
> +ssize_t __kernel_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len);
> +
> +ssize_t __kernel_getrandom(void *buffer, size_t len, unsigned int flags, void *opaque_state, size_t opaque_len)
> +{
> +	return __cvdso_getrandom(buffer, len, flags, opaque_state, opaque_len);
> +}
> diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
> index 10ffdda3f2fa..f07ea679a4cc 100644
> --- a/tools/testing/selftests/vDSO/Makefile
> +++ b/tools/testing/selftests/vDSO/Makefile
> @@ -1,6 +1,6 @@
>   # 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/)
> +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)))

Does that work for you when you cross-compile ? For powerpc when I cross 
compile I still get the x86_64 from uname_M here, which is unexpected.

>   TEST_GEN_PROGS += vdso_test_getrandom
>   ifneq ($(SODIUM),)
>   TEST_GEN_PROGS += vdso_test_chacha

Christophe

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ