Re: [PATCH v17 5/5] x86: vdso: Wire up getrandom() vDSO implementation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/14/24 12:06 PM, Jason A. Donenfeld wrote:
Hook up the generic vDSO implementation to the x86 vDSO data page. Since
the existing vDSO infrastructure is heavily based on the timekeeping
functionality, which works over arrays of bases, a new macro is
introduced for vvars that are not arrays.

The vDSO function requires a ChaCha20 implementation that does not write
to the stack, yet can still do an entire ChaCha20 permutation, so
provide this using SSE2, since this is userland code that must work on
all x86-64 processors. There's a simple test for this code as well.

Reviewed-by: Samuel Neves <sneves@xxxxxxxxx> # for vgetrandom-chacha.S
Signed-off-by: Jason A. Donenfeld <Jason@xxxxxxxxx>
---
  arch/x86/Kconfig                              |   1 +
  arch/x86/entry/vdso/Makefile                  |   3 +-
  arch/x86/entry/vdso/vdso.lds.S                |   2 +
  arch/x86/entry/vdso/vgetrandom-chacha.S       | 178 ++++++++++++++++++
  arch/x86/entry/vdso/vgetrandom.c              |  17 ++
  arch/x86/include/asm/vdso/getrandom.h         |  55 ++++++
  arch/x86/include/asm/vdso/vsyscall.h          |   2 +
  arch/x86/include/asm/vvar.h                   |  16 ++
  tools/testing/selftests/vDSO/.gitignore       |   1 +
  tools/testing/selftests/vDSO/Makefile         |  13 ++
  .../testing/selftests/vDSO/vdso_test_chacha.c |  43 +++++

Hi Jason,

This is a large patch, so it might be helpful to split out the selftests
into their own patch. In fact, my comments here are only about those.

I'm adding linux-kselftest to Cc for visibility, and I've also Cc'd you
on a related selftests/vDSO series I just now posted [1].

In fact, I think it might work well if you insert patches 2/3 and 3/3
from that series, and build on top of those for the
selftests/vDSO/Makefile. See below for details.

...

diff --git a/tools/testing/selftests/vDSO/Makefile b/tools/testing/selftests/vDSO/Makefile
index a33b4d200a32..8b87ebea1630 100644
--- a/tools/testing/selftests/vDSO/Makefile
+++ b/tools/testing/selftests/vDSO/Makefile
@@ -3,6 +3,7 @@ include ../lib.mk
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)
TEST_GEN_PROGS := $(OUTPUT)/vdso_test_gettimeofday $(OUTPUT)/vdso_test_getcpu
  TEST_GEN_PROGS += $(OUTPUT)/vdso_test_abi
@@ -12,9 +13,15 @@ TEST_GEN_PROGS += $(OUTPUT)/vdso_standalone_test_x86
  endif
  TEST_GEN_PROGS += $(OUTPUT)/vdso_test_correctness
  TEST_GEN_PROGS += $(OUTPUT)/vdso_test_getrandom
+ifeq ($(uname_M),x86_64)
+ifneq ($(SODIUM),)
+TEST_GEN_PROGS += $(OUTPUT)/vdso_test_chacha

Unfortunately, this is "pre-existing wrong". :) That is, it is following
a pre-existing pattern that is broken: the $(OUTPUT) is not supposed to
be part of TEST_GEN_PROGS. Fixing it requires other changes, though, as
I've done in [2].


[1] https://lore.kernel.org/all/20240614233105.265009-1-jhubbard@xxxxxxxxxx/
[2] https://lore.kernel.org/all/20240614233105.265009-3-jhubbard@xxxxxxxxxx/
[3] https://lore.kernel.org/all/20240614233105.265009-4-jhubbard@xxxxxxxxxx/

+endif
+endif
CFLAGS := -std=gnu99
  CFLAGS_vdso_standalone_test_x86 := -nostdlib -fno-asynchronous-unwind-tables -fno-stack-protector
+CFLAGS_vdso_test_chacha := $(SODIUM) -idirafter $(top_srcdir)/include -idirafter $(top_srcdir)/arch/$(ARCH)/include -idirafter include -D__ASSEMBLY__ -DBULID_VDSO -DCONFIG_FUNCTION_ALIGNMENT=0 -Wa,--noexecstack

Line breaks via "\" are allowed in Makefiles. Might need two or three here.

  LDFLAGS_vdso_test_correctness := -ldl
  ifeq ($(CONFIG_X86_32),y)
  LDLIBS += -lgcc_s
@@ -35,3 +42,9 @@ $(OUTPUT)/vdso_test_correctness: vdso_test_correctness.c
  		-o $@ \
  		$(LDFLAGS_vdso_test_correctness)
  $(OUTPUT)/vdso_test_getrandom: parse_vdso.c
+$(OUTPUT)/vdso_test_chacha: CFLAGS += $(CFLAGS_vdso_test_chacha)

This also follows an unfortunate pattern, which I've also fixed just today
in a patch [3]. Please just add to CFLAGS directly, rather than creating
these name-mangled intermediate variables.  See [3] for how that looks.

+$(OUTPUT)/vdso_test_chacha: $(top_srcdir)/arch/$(ARCH)/entry/vdso/vgetrandom-chacha.S
+$(OUTPUT)/vdso_test_chacha: include/asm/rwonce.h
+include/asm/rwonce.h:
+	mkdir -p include/asm
+	touch $@



thanks,
--
John Hubbard
NVIDIA





[Index of Archives]     [Kernel]     [Gnu Classpath]     [Gnu Crypto]     [DM Crypt]     [Netfilter]     [Bugtraq]
  Powered by Linux