linux-kernel - Re: [PATCH] selftests: vDSO: Do not rely on $ARCH for vdso_test_getrandom && vdso_test

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: <667622ae-dde5-410f-a9f8-4801788af278@csgroup.eu>
Date: Sun, 1 Sep 2024 20:00:30 +0200
From: Christophe Leroy <christophe.leroy@...roup.eu>
To: "Jason A. Donenfeld" <Jason@...c4.com>
Cc: linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
 Adhemerval Zanella <adhemerval.zanella@...aro.org>,
 Xi Ruoyao <xry111@...111.site>, broonie@...nel.org
Subject: Re: [PATCH] selftests: vDSO: Do not rely on $ARCH for
 vdso_test_getrandom && vdso_test_chacha

Hi Jason,

Le 01/09/2024 à 15:22, Jason A. Donenfeld a écrit :
> Hi Christophe,
> 
> Hmm, I'm not so sure I like this very much. I think it's important for
> these tests to fail when an arch tries to hook up the function to the
> vDSO, but it's still not exported for some reason. This also regresses
> the ARCH=x86_64 vs ARCH=x86 thing, which SRCARCH fixes.
> 
> What about, instead, something like below, replacing the other commit?

I need to look at it in more details and perfom a test, but after first 
look I can't figure out how it would work.

When I build selftests,

to build 32 bits selftests I do:

	make ARCH=powerpc CROSS_COMPILE=ppc-linux-

to build a 64 bits BE selftests I do:

	make ARCH=powerpc CROSS_COMPILE=powerpc64-linux-

to build a 64 bits LE selftests I do:

	make ARCH=powerpc CROSS_COMPILE=powerpc64le-linux-


I addition, in case someone does the build on a native platform directly,

On 32 bits, uname -m returns 'ppc'
On 64 bits, uname -m returns 'ppc64'
On 64 bits little endian, uname -m returns 'ppc64le'

How would this fit in the logic where IIUC you just remove '_64' from 
'x86_64' to get 'x86'

Christophe

> 
> Jason
> 
>  From ccc53425c98f4f5c2a1edaf775089efb56bd106e Mon Sep 17 00:00:00 2001
> From: "Jason A. Donenfeld" <Jason@...c4.com>
> Date: Sun, 1 Sep 2024 15:05:01 +0200
> Subject: [PATCH] selftests: vDSO: fix cross build for getrandom and chacha
>   tests
> 
> Unlike the check for the standalone x86 test, the check for building the
> vDSO getrandom and chacaha tests looks at the architecture for the host
> rather than the architecture for the target when deciding if they should
> be built. Since the chacha test includes some assembler code this means
> that cross building with x86 as either the target or host is broken.
> 
> There's also some additional complications, where ARCH can legitimately
> be either x86_64 or x86, but the source code we need to compile lives in
> a directory path containing arch/x86. The standard SRCARCH variable
> handles that. And actually, all these variables and proper substitutions
> are already described in tools/scripts/Makefile.arch, so just include
> that to handle it.
> 
> Similarly, ARCH=x86 can actually describe ARCH=x86_64,
> just with CONFIG_64BIT, so we can't rely on ARCH for selecting
> non-32-bit tests. For that, check against $(ARCH)$(CONFIG_X86_32). This
> won't help for people manually running this inside the vDSO selftest
> directory (which isn't really supported anyway and has problems on
> various archs), but it should work for builds of the kselftests, where
> the CONFIG_* variables are defined. On x86_64 machines,
> $(ARCH)$(CONFIG_X86_32) will evaluate to x86. On arm64 machines, it will
> evaluate to arm64. On 32-bit x86 machines, it will evaluate to x86y,
> which won't match the filter list.
> 
> Reported-by: Mark Brown <broonie@...nel.org>
> Reported-by: Christophe Leroy <christophe.leroy@...roup.eu>
> Signed-off-by: Jason A. Donenfeld <Jason@...c4.com>
> ---
>   tools/testing/selftests/vDSO/Makefile | 9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
> index e21e78aae24d..01a5805062b3 100644
> --- a/tools/testing/selftests/vDSO/Makefile
> +++ b/tools/testing/selftests/vDSO/Makefile
> @@ -1,6 +1,5 @@
>   # 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/)
> +include ../../../scripts/Makefile.arch
> 
>   TEST_GEN_PROGS := vdso_test_gettimeofday
>   TEST_GEN_PROGS += vdso_test_getcpu
> @@ -10,7 +9,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 ($(ARCH)$(CONFIG_X86_32),$(filter $(ARCH)$(CONFIG_X86_32),x86))
>   TEST_GEN_PROGS += vdso_test_getrandom
>   TEST_GEN_PROGS += vdso_test_chacha
>   endif
> @@ -38,8 +37,8 @@ $(OUTPUT)/vdso_test_getrandom: CFLAGS += -isystem $(top_srcdir)/tools/include \
>                                            $(KHDR_INCLUDES) \
>                                            -isystem $(top_srcdir)/include/uapi
> 
> -$(OUTPUT)/vdso_test_chacha: $(top_srcdir)/tools/arch/$(ARCH)/vdso/vgetrandom-chacha.S
> +$(OUTPUT)/vdso_test_chacha: $(top_srcdir)/tools/arch/$(SRCARCH)/vdso/vgetrandom-chacha.S
>   $(OUTPUT)/vdso_test_chacha: CFLAGS += -idirafter $(top_srcdir)/tools/include \
> -                                      -idirafter $(top_srcdir)/arch/$(ARCH)/include \
> +                                      -idirafter $(top_srcdir)/arch/$(SRCARCH)/include \
>                                         -idirafter $(top_srcdir)/include \
>                                         -D__ASSEMBLY__ -Wa,--noexecstack
> --
> 2.46.0
> 
>