linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] vdso: Reject absolute relocations during build
@ 2025-06-11  9:22 Thomas Weißschuh
  2025-06-11  9:22 ` [PATCH v3 1/3] riscv: vdso: Deduplicate CFLAGS_REMOVE_* variables Thomas Weißschuh
                   ` (2 more replies)
  0 siblings, 3 replies; 15+ messages in thread
From: Thomas Weißschuh @ 2025-06-11  9:22 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino
  Cc: linux-riscv, linux-kernel, llvm, Thomas Weißschuh

The compiler can emit absolute relocations in vDSO code,
which are invalid in vDSO code.
Detect them at compile-time.

Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
Changes in v3:
- Drop already applied bugfix for arm64
- Disable LTO for the riscv vDSO, as it is incompatible
- Link to v2: https://lore.kernel.org/r/20250430-vdso-absolute-reloc-v2-0-5efcc3bc4b26@linutronix.de

Changes in v2:
- Link to openend (invalid) GCC bug containing more explanations
- Refine commit messages
- Don't fail on commit absolute relocations in debug info
- Link to v1: https://lore.kernel.org/r/20250429-vdso-absolute-reloc-v1-0-987a0afd10b5@linutronix.de

---
Thomas Weißschuh (3):
      riscv: vdso: Deduplicate CFLAGS_REMOVE_* variables
      riscv: vdso: Disable LTO for the vDSO
      vdso: Reject absolute relocations during build

 arch/riscv/kernel/vdso/Makefile | 7 ++++---
 lib/vdso/Makefile.include       | 6 ++++++
 2 files changed, 10 insertions(+), 3 deletions(-)
---
base-commit: 13a2ea925ad717de32b5aeaaccda62ed26146d9f
change-id: 20250428-vdso-absolute-reloc-a226293c1761

Best regards,
-- 
Thomas Weißschuh <thomas.weissschuh@linutronix.de>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v3 1/3] riscv: vdso: Deduplicate CFLAGS_REMOVE_* variables
  2025-06-11  9:22 [PATCH v3 0/3] vdso: Reject absolute relocations during build Thomas Weißschuh
@ 2025-06-11  9:22 ` Thomas Weißschuh
  2025-06-11 18:40   ` Nathan Chancellor
  2025-06-11  9:22 ` [PATCH v3 2/3] riscv: vdso: Disable LTO for the vDSO Thomas Weißschuh
  2025-06-11  9:22 ` [PATCH v3 3/3] vdso: Reject absolute relocations during build Thomas Weißschuh
  2 siblings, 1 reply; 15+ messages in thread
From: Thomas Weißschuh @ 2025-06-11  9:22 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino
  Cc: linux-riscv, linux-kernel, llvm, Thomas Weißschuh

All vDSO objects need the same treatment.
To make changes to both the list of objects and the list of removed flags
easier, introduce a helper variable.

Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
 arch/riscv/kernel/vdso/Makefile | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
index 9ebb5e590f93a3228c451dca58e6d5cfbbc03ff7..c19c3c76f7c9f6b2f7523a59269de3b683656323 100644
--- a/arch/riscv/kernel/vdso/Makefile
+++ b/arch/riscv/kernel/vdso/Makefile
@@ -49,9 +49,10 @@ CPPFLAGS_vdso.lds += -DHAS_VGETTIMEOFDAY
 endif
 
 # Disable -pg to prevent insert call site
-CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
-CFLAGS_REMOVE_getrandom.o = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
-CFLAGS_REMOVE_hwprobe.o = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
+CFLAGS_REMOVE_VDSO = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
+CFLAGS_REMOVE_vgettimeofday.o = $(CFLAGS_REMOVE_VDSO)
+CFLAGS_REMOVE_getrandom.o = $(CFLAGS_REMOVE_VDSO)
+CFLAGS_REMOVE_hwprobe.o = $(CFLAGS_REMOVE_VDSO)
 
 # Force dependency
 $(obj)/vdso.o: $(obj)/vdso.so

-- 
2.49.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 2/3] riscv: vdso: Disable LTO for the vDSO
  2025-06-11  9:22 [PATCH v3 0/3] vdso: Reject absolute relocations during build Thomas Weißschuh
  2025-06-11  9:22 ` [PATCH v3 1/3] riscv: vdso: Deduplicate CFLAGS_REMOVE_* variables Thomas Weißschuh
@ 2025-06-11  9:22 ` Thomas Weißschuh
  2025-06-11 18:41   ` Nathan Chancellor
  2025-06-11  9:22 ` [PATCH v3 3/3] vdso: Reject absolute relocations during build Thomas Weißschuh
  2 siblings, 1 reply; 15+ messages in thread
From: Thomas Weißschuh @ 2025-06-11  9:22 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino
  Cc: linux-riscv, linux-kernel, llvm, Thomas Weißschuh

All the other architectures supporting LTO (x86, arm64, loongarch) do not
use it for the vDSO.

Its is problematic for some upcoming compile-time validation of the
generated object code.
The LTO object files do not contain the necessary relocation information
and -flto-fat-objects is not compatible with clang < 16.

For consistency and to enable the mentioned compile-time checks,
disable LTO for the vDSO.
The vDSO heavily uses __always_inline anyways.

Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
 arch/riscv/kernel/vdso/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
index c19c3c76f7c9f6b2f7523a59269de3b683656323..9f1bf5bae9bd473e43d9fd3022e9e1a185128b73 100644
--- a/arch/riscv/kernel/vdso/Makefile
+++ b/arch/riscv/kernel/vdso/Makefile
@@ -49,7 +49,7 @@ CPPFLAGS_vdso.lds += -DHAS_VGETTIMEOFDAY
 endif
 
 # Disable -pg to prevent insert call site
-CFLAGS_REMOVE_VDSO = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
+CFLAGS_REMOVE_VDSO = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS) $(CC_FLAGS_LTO)
 CFLAGS_REMOVE_vgettimeofday.o = $(CFLAGS_REMOVE_VDSO)
 CFLAGS_REMOVE_getrandom.o = $(CFLAGS_REMOVE_VDSO)
 CFLAGS_REMOVE_hwprobe.o = $(CFLAGS_REMOVE_VDSO)

-- 
2.49.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v3 3/3] vdso: Reject absolute relocations during build
  2025-06-11  9:22 [PATCH v3 0/3] vdso: Reject absolute relocations during build Thomas Weißschuh
  2025-06-11  9:22 ` [PATCH v3 1/3] riscv: vdso: Deduplicate CFLAGS_REMOVE_* variables Thomas Weißschuh
  2025-06-11  9:22 ` [PATCH v3 2/3] riscv: vdso: Disable LTO for the vDSO Thomas Weißschuh
@ 2025-06-11  9:22 ` Thomas Weißschuh
  2025-06-11 18:57   ` Nathan Chancellor
  2025-06-12  8:31   ` Alexandre Ghiti
  2 siblings, 2 replies; 15+ messages in thread
From: Thomas Weißschuh @ 2025-06-11  9:22 UTC (permalink / raw)
  To: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino
  Cc: linux-riscv, linux-kernel, llvm, Thomas Weißschuh

All vDSO code needs to be completely position independent.
Symbol references are marked as hidden so the compiler emits
PC-relative relocations. However there are cases where the compiler may
still emit absolute relocations, as they are valid in regular PIC DSO code.
These would be resolved by the linker and will break at runtime.
This has been observed on arm64 under some circumstances, see
commit 0c314cda9325 ("arm64: vdso: Work around invalid absolute relocations from GCC")

Introduce a build-time check for absolute relocations.
The check is done on the object files as the relocations will not exist
anymore in the final DSO. As there is no extension point for the
compilation of each object file, perform the validation in vdso_check.

Debug information can contain legal absolute relocations and readelf can
not print relocations from the .text section only. Make use of the fact
that all global vDSO symbols follow the naming pattern "vdso_u_".

Link: https://lore.kernel.org/lkml/aApGPAoctq_eoE2g@t14ultra/
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120002
Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
---
 lib/vdso/Makefile.include | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/vdso/Makefile.include b/lib/vdso/Makefile.include
index cedbf15f80874d4bb27c097244bc5b11272f261c..04257d0f28c0ed324e31adbb68497181085752f8 100644
--- a/lib/vdso/Makefile.include
+++ b/lib/vdso/Makefile.include
@@ -12,7 +12,13 @@ c-getrandom-$(CONFIG_VDSO_GETRANDOM) := $(addprefix $(GENERIC_VDSO_DIR), getrand
 #
 # As a workaround for some GNU ld ports which produce unneeded R_*_NONE
 # dynamic relocations, ignore R_*_NONE.
+#
+# Also validate that no absolute relocations against global symbols are present
+# in the object files.
 quiet_cmd_vdso_check = VDSOCHK $@
       cmd_vdso_check = if $(READELF) -rW $@ | grep -v _NONE | grep -q " R_\w*_"; \
 		       then (echo >&2 "$@: dynamic relocations are not supported"; \
+			     rm -f $@; /bin/false); fi && \
+		       if $(READELF) -rW $(filter %.o, $(real-prereqs)) | grep -q " R_\w*_ABS.*vdso_u_"; \
+		       then (echo >&2 "$@: absolute relocations are not supported"; \
 			     rm -f $@; /bin/false); fi

-- 
2.49.0


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/3] riscv: vdso: Deduplicate CFLAGS_REMOVE_* variables
  2025-06-11  9:22 ` [PATCH v3 1/3] riscv: vdso: Deduplicate CFLAGS_REMOVE_* variables Thomas Weißschuh
@ 2025-06-11 18:40   ` Nathan Chancellor
  2025-06-12  8:13     ` Alexandre Ghiti
  0 siblings, 1 reply; 15+ messages in thread
From: Nathan Chancellor @ 2025-06-11 18:40 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Nick Desaulniers, Bill Wendling, Justin Stitt, Andy Lutomirski,
	Thomas Gleixner, Vincenzo Frascino, linux-riscv, linux-kernel,
	llvm

On Wed, Jun 11, 2025 at 11:22:12AM +0200, Thomas Weißschuh wrote:
> All vDSO objects need the same treatment.
> To make changes to both the list of objects and the list of removed flags
> easier, introduce a helper variable.
> 
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  arch/riscv/kernel/vdso/Makefile | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
> index 9ebb5e590f93a3228c451dca58e6d5cfbbc03ff7..c19c3c76f7c9f6b2f7523a59269de3b683656323 100644
> --- a/arch/riscv/kernel/vdso/Makefile
> +++ b/arch/riscv/kernel/vdso/Makefile
> @@ -49,9 +49,10 @@ CPPFLAGS_vdso.lds += -DHAS_VGETTIMEOFDAY
>  endif
>  
>  # Disable -pg to prevent insert call site
> -CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
> -CFLAGS_REMOVE_getrandom.o = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
> -CFLAGS_REMOVE_hwprobe.o = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
> +CFLAGS_REMOVE_VDSO = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
> +CFLAGS_REMOVE_vgettimeofday.o = $(CFLAGS_REMOVE_VDSO)
> +CFLAGS_REMOVE_getrandom.o = $(CFLAGS_REMOVE_VDSO)
> +CFLAGS_REMOVE_hwprobe.o = $(CFLAGS_REMOVE_VDSO)
>  
>  # Force dependency
>  $(obj)/vdso.o: $(obj)/vdso.so
> 
> -- 
> 2.49.0
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 2/3] riscv: vdso: Disable LTO for the vDSO
  2025-06-11  9:22 ` [PATCH v3 2/3] riscv: vdso: Disable LTO for the vDSO Thomas Weißschuh
@ 2025-06-11 18:41   ` Nathan Chancellor
  0 siblings, 0 replies; 15+ messages in thread
From: Nathan Chancellor @ 2025-06-11 18:41 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Nick Desaulniers, Bill Wendling, Justin Stitt, Andy Lutomirski,
	Thomas Gleixner, Vincenzo Frascino, linux-riscv, linux-kernel,
	llvm

On Wed, Jun 11, 2025 at 11:22:13AM +0200, Thomas Weißschuh wrote:
> All the other architectures supporting LTO (x86, arm64, loongarch) do not
> use it for the vDSO.
> 
> Its is problematic for some upcoming compile-time validation of the
> generated object code.
> The LTO object files do not contain the necessary relocation information
> and -flto-fat-objects is not compatible with clang < 16.
> 
> For consistency and to enable the mentioned compile-time checks,
> disable LTO for the vDSO.
> The vDSO heavily uses __always_inline anyways.
> 
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>

Yes, I would agree this has little actual value for the vDSO.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  arch/riscv/kernel/vdso/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
> index c19c3c76f7c9f6b2f7523a59269de3b683656323..9f1bf5bae9bd473e43d9fd3022e9e1a185128b73 100644
> --- a/arch/riscv/kernel/vdso/Makefile
> +++ b/arch/riscv/kernel/vdso/Makefile
> @@ -49,7 +49,7 @@ CPPFLAGS_vdso.lds += -DHAS_VGETTIMEOFDAY
>  endif
>  
>  # Disable -pg to prevent insert call site
> -CFLAGS_REMOVE_VDSO = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
> +CFLAGS_REMOVE_VDSO = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS) $(CC_FLAGS_LTO)
>  CFLAGS_REMOVE_vgettimeofday.o = $(CFLAGS_REMOVE_VDSO)
>  CFLAGS_REMOVE_getrandom.o = $(CFLAGS_REMOVE_VDSO)
>  CFLAGS_REMOVE_hwprobe.o = $(CFLAGS_REMOVE_VDSO)
> 
> -- 
> 2.49.0
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/3] vdso: Reject absolute relocations during build
  2025-06-11  9:22 ` [PATCH v3 3/3] vdso: Reject absolute relocations during build Thomas Weißschuh
@ 2025-06-11 18:57   ` Nathan Chancellor
  2025-06-12  5:48     ` Thomas Weißschuh
  2025-06-12  8:31   ` Alexandre Ghiti
  1 sibling, 1 reply; 15+ messages in thread
From: Nathan Chancellor @ 2025-06-11 18:57 UTC (permalink / raw)
  To: Thomas Weißschuh
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Nick Desaulniers, Bill Wendling, Justin Stitt, Andy Lutomirski,
	Thomas Gleixner, Vincenzo Frascino, linux-riscv, linux-kernel,
	llvm

On Wed, Jun 11, 2025 at 11:22:14AM +0200, Thomas Weißschuh wrote:
> All vDSO code needs to be completely position independent.
> Symbol references are marked as hidden so the compiler emits
> PC-relative relocations. However there are cases where the compiler may
> still emit absolute relocations, as they are valid in regular PIC DSO code.
> These would be resolved by the linker and will break at runtime.
> This has been observed on arm64 under some circumstances, see
> commit 0c314cda9325 ("arm64: vdso: Work around invalid absolute relocations from GCC")
> 
> Introduce a build-time check for absolute relocations.
> The check is done on the object files as the relocations will not exist
> anymore in the final DSO. As there is no extension point for the
> compilation of each object file, perform the validation in vdso_check.
> 
> Debug information can contain legal absolute relocations and readelf can
> not print relocations from the .text section only. Make use of the fact
> that all global vDSO symbols follow the naming pattern "vdso_u_".
> 
> Link: https://lore.kernel.org/lkml/aApGPAoctq_eoE2g@t14ultra/
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120002
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>

I ran this through a few different architectures with LLVM=1 and did not
see anything interesting.

Reviewed-by: Nathan Chancellor <nathan@kernel.org>
Tested-by: Nathan Chancellor <nathan@kernel.org>

> ---
>  lib/vdso/Makefile.include | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/lib/vdso/Makefile.include b/lib/vdso/Makefile.include
> index cedbf15f80874d4bb27c097244bc5b11272f261c..04257d0f28c0ed324e31adbb68497181085752f8 100644
> --- a/lib/vdso/Makefile.include
> +++ b/lib/vdso/Makefile.include
> @@ -12,7 +12,13 @@ c-getrandom-$(CONFIG_VDSO_GETRANDOM) := $(addprefix $(GENERIC_VDSO_DIR), getrand
>  #
>  # As a workaround for some GNU ld ports which produce unneeded R_*_NONE
>  # dynamic relocations, ignore R_*_NONE.
> +#
> +# Also validate that no absolute relocations against global symbols are present
> +# in the object files.
>  quiet_cmd_vdso_check = VDSOCHK $@

I do not see VDSOCHK in the normal build log but I do see the check
being executed with V=1. That's obviously an outstanding issue but
figured it was worth mentioning.

>        cmd_vdso_check = if $(READELF) -rW $@ | grep -v _NONE | grep -q " R_\w*_"; \
>  		       then (echo >&2 "$@: dynamic relocations are not supported"; \
> +			     rm -f $@; /bin/false); fi && \
> +		       if $(READELF) -rW $(filter %.o, $(real-prereqs)) | grep -q " R_\w*_ABS.*vdso_u_"; \
> +		       then (echo >&2 "$@: absolute relocations are not supported"; \
>  			     rm -f $@; /bin/false); fi
> 
> -- 
> 2.49.0
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/3] vdso: Reject absolute relocations during build
  2025-06-11 18:57   ` Nathan Chancellor
@ 2025-06-12  5:48     ` Thomas Weißschuh
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Weißschuh @ 2025-06-12  5:48 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Alexandre Ghiti,
	Nick Desaulniers, Bill Wendling, Justin Stitt, Andy Lutomirski,
	Thomas Gleixner, Vincenzo Frascino, linux-riscv, linux-kernel,
	llvm

On Wed, Jun 11, 2025 at 11:57:27AM -0700, Nathan Chancellor wrote:
> On Wed, Jun 11, 2025 at 11:22:14AM +0200, Thomas Weißschuh wrote:
> > All vDSO code needs to be completely position independent.
> > Symbol references are marked as hidden so the compiler emits
> > PC-relative relocations. However there are cases where the compiler may
> > still emit absolute relocations, as they are valid in regular PIC DSO code.
> > These would be resolved by the linker and will break at runtime.
> > This has been observed on arm64 under some circumstances, see
> > commit 0c314cda9325 ("arm64: vdso: Work around invalid absolute relocations from GCC")
> > 
> > Introduce a build-time check for absolute relocations.
> > The check is done on the object files as the relocations will not exist
> > anymore in the final DSO. As there is no extension point for the
> > compilation of each object file, perform the validation in vdso_check.
> > 
> > Debug information can contain legal absolute relocations and readelf can
> > not print relocations from the .text section only. Make use of the fact
> > that all global vDSO symbols follow the naming pattern "vdso_u_".
> > 
> > Link: https://lore.kernel.org/lkml/aApGPAoctq_eoE2g@t14ultra/
> > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120002
> > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> 
> I ran this through a few different architectures with LLVM=1 and did not
> see anything interesting.
> 
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
> Tested-by: Nathan Chancellor <nathan@kernel.org>

Thanks!

> > ---
> >  lib/vdso/Makefile.include | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/lib/vdso/Makefile.include b/lib/vdso/Makefile.include
> > index cedbf15f80874d4bb27c097244bc5b11272f261c..04257d0f28c0ed324e31adbb68497181085752f8 100644
> > --- a/lib/vdso/Makefile.include
> > +++ b/lib/vdso/Makefile.include
> > @@ -12,7 +12,13 @@ c-getrandom-$(CONFIG_VDSO_GETRANDOM) := $(addprefix $(GENERIC_VDSO_DIR), getrand
> >  #
> >  # As a workaround for some GNU ld ports which produce unneeded R_*_NONE
> >  # dynamic relocations, ignore R_*_NONE.
> > +#
> > +# Also validate that no absolute relocations against global symbols are present
> > +# in the object files.
> >  quiet_cmd_vdso_check = VDSOCHK $@
> 
> I do not see VDSOCHK in the normal build log but I do see the check
> being executed with V=1. That's obviously an outstanding issue but
> figured it was worth mentioning.

This is because nobody actually directly uses the command "vdso_check".
Instead all users use the variable "cmd_vdso_check" to build their own command:

quiet_cmd_vdso_and_check = VDSO    $@
      cmd_vdso_and_check = $(cmd_vdso); $(cmd_vdso_check)

$(obj)/vdso64.so.dbg: $(obj)/vdso.lds $(vobjs) FORCE
	$(call if_changed,vdso_and_check)

So you will only see "VDSO" in the logs.
Which makes sense as VDSOCHK does not produce any output, so integrating it as
its own command would be iffy.

> >        cmd_vdso_check = if $(READELF) -rW $@ | grep -v _NONE | grep -q " R_\w*_"; \
> >  		       then (echo >&2 "$@: dynamic relocations are not supported"; \
> > +			     rm -f $@; /bin/false); fi && \
> > +		       if $(READELF) -rW $(filter %.o, $(real-prereqs)) | grep -q " R_\w*_ABS.*vdso_u_"; \
> > +		       then (echo >&2 "$@: absolute relocations are not supported"; \
> >  			     rm -f $@; /bin/false); fi
> > 
> > -- 
> > 2.49.0
> > 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 1/3] riscv: vdso: Deduplicate CFLAGS_REMOVE_* variables
  2025-06-11 18:40   ` Nathan Chancellor
@ 2025-06-12  8:13     ` Alexandre Ghiti
  0 siblings, 0 replies; 15+ messages in thread
From: Alexandre Ghiti @ 2025-06-12  8:13 UTC (permalink / raw)
  To: Nathan Chancellor, Thomas Weißschuh
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Nick Desaulniers,
	Bill Wendling, Justin Stitt, Andy Lutomirski, Thomas Gleixner,
	Vincenzo Frascino, linux-riscv, linux-kernel, llvm

Hi Thomas,

On 6/11/25 20:40, Nathan Chancellor wrote:
> On Wed, Jun 11, 2025 at 11:22:12AM +0200, Thomas Weißschuh wrote:
>> All vDSO objects need the same treatment.
>> To make changes to both the list of objects and the list of removed flags
>> easier, introduce a helper variable.
>>
>> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>


Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex


>
>> ---
>>   arch/riscv/kernel/vdso/Makefile | 7 ++++---
>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/riscv/kernel/vdso/Makefile b/arch/riscv/kernel/vdso/Makefile
>> index 9ebb5e590f93a3228c451dca58e6d5cfbbc03ff7..c19c3c76f7c9f6b2f7523a59269de3b683656323 100644
>> --- a/arch/riscv/kernel/vdso/Makefile
>> +++ b/arch/riscv/kernel/vdso/Makefile
>> @@ -49,9 +49,10 @@ CPPFLAGS_vdso.lds += -DHAS_VGETTIMEOFDAY
>>   endif
>>   
>>   # Disable -pg to prevent insert call site
>> -CFLAGS_REMOVE_vgettimeofday.o = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
>> -CFLAGS_REMOVE_getrandom.o = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
>> -CFLAGS_REMOVE_hwprobe.o = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
>> +CFLAGS_REMOVE_VDSO = $(CC_FLAGS_FTRACE) $(CC_FLAGS_SCS)
>> +CFLAGS_REMOVE_vgettimeofday.o = $(CFLAGS_REMOVE_VDSO)
>> +CFLAGS_REMOVE_getrandom.o = $(CFLAGS_REMOVE_VDSO)
>> +CFLAGS_REMOVE_hwprobe.o = $(CFLAGS_REMOVE_VDSO)
>>   
>>   # Force dependency
>>   $(obj)/vdso.o: $(obj)/vdso.so
>>
>> -- 
>> 2.49.0
>>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/3] vdso: Reject absolute relocations during build
  2025-06-11  9:22 ` [PATCH v3 3/3] vdso: Reject absolute relocations during build Thomas Weißschuh
  2025-06-11 18:57   ` Nathan Chancellor
@ 2025-06-12  8:31   ` Alexandre Ghiti
  2025-06-12 14:21     ` Thomas Weißschuh
  2025-06-12 20:05     ` Palmer Dabbelt
  1 sibling, 2 replies; 15+ messages in thread
From: Alexandre Ghiti @ 2025-06-12  8:31 UTC (permalink / raw)
  To: Thomas Weißschuh, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	Andy Lutomirski, Thomas Gleixner, Vincenzo Frascino
  Cc: linux-riscv, linux-kernel, llvm

Hi Thomas,

On 6/11/25 11:22, Thomas Weißschuh wrote:
> All vDSO code needs to be completely position independent.
> Symbol references are marked as hidden so the compiler emits
> PC-relative relocations. However there are cases where the compiler may
> still emit absolute relocations, as they are valid in regular PIC DSO code.
> These would be resolved by the linker and will break at runtime.
> This has been observed on arm64 under some circumstances, see
> commit 0c314cda9325 ("arm64: vdso: Work around invalid absolute relocations from GCC")
>
> Introduce a build-time check for absolute relocations.
> The check is done on the object files as the relocations will not exist
> anymore in the final DSO. As there is no extension point for the
> compilation of each object file, perform the validation in vdso_check.
>
> Debug information can contain legal absolute relocations and readelf can
> not print relocations from the .text section only. Make use of the fact
> that all global vDSO symbols follow the naming pattern "vdso_u_".
>
> Link: https://lore.kernel.org/lkml/aApGPAoctq_eoE2g@t14ultra/
> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120002
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>   lib/vdso/Makefile.include | 6 ++++++
>   1 file changed, 6 insertions(+)
>
> diff --git a/lib/vdso/Makefile.include b/lib/vdso/Makefile.include
> index cedbf15f80874d4bb27c097244bc5b11272f261c..04257d0f28c0ed324e31adbb68497181085752f8 100644
> --- a/lib/vdso/Makefile.include
> +++ b/lib/vdso/Makefile.include
> @@ -12,7 +12,13 @@ c-getrandom-$(CONFIG_VDSO_GETRANDOM) := $(addprefix $(GENERIC_VDSO_DIR), getrand
>   #
>   # As a workaround for some GNU ld ports which produce unneeded R_*_NONE
>   # dynamic relocations, ignore R_*_NONE.
> +#
> +# Also validate that no absolute relocations against global symbols are present
> +# in the object files.
>   quiet_cmd_vdso_check = VDSOCHK $@
>         cmd_vdso_check = if $(READELF) -rW $@ | grep -v _NONE | grep -q " R_\w*_"; \
>   		       then (echo >&2 "$@: dynamic relocations are not supported"; \
> +			     rm -f $@; /bin/false); fi && \
> +		       if $(READELF) -rW $(filter %.o, $(real-prereqs)) | grep -q " R_\w*_ABS.*vdso_u_"; \


This only works for arm64 relocations right? I can't find any *ABS* 
relocations in riscv elf abi 
(https://github.com/riscv-non-isa/riscv-elf-psabi-doc/releases/tag/v1.0).

Thanks,

Alex


> +		       then (echo >&2 "$@: absolute relocations are not supported"; \
>   			     rm -f $@; /bin/false); fi
>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/3] vdso: Reject absolute relocations during build
  2025-06-12  8:31   ` Alexandre Ghiti
@ 2025-06-12 14:21     ` Thomas Weißschuh
  2025-06-21 15:42       ` Thomas Gleixner
  2025-06-12 20:05     ` Palmer Dabbelt
  1 sibling, 1 reply; 15+ messages in thread
From: Thomas Weißschuh @ 2025-06-12 14:21 UTC (permalink / raw)
  To: Alexandre Ghiti
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt, Andy Lutomirski,
	Thomas Gleixner, Vincenzo Frascino, linux-riscv, linux-kernel,
	llvm

Hi Alexandre,

On Thu, Jun 12, 2025 at 10:31:20AM +0200, Alexandre Ghiti wrote:
> On 6/11/25 11:22, Thomas Weißschuh wrote:
> > All vDSO code needs to be completely position independent.
> > Symbol references are marked as hidden so the compiler emits
> > PC-relative relocations. However there are cases where the compiler may
> > still emit absolute relocations, as they are valid in regular PIC DSO code.
> > These would be resolved by the linker and will break at runtime.
> > This has been observed on arm64 under some circumstances, see
> > commit 0c314cda9325 ("arm64: vdso: Work around invalid absolute relocations from GCC")
> > 
> > Introduce a build-time check for absolute relocations.
> > The check is done on the object files as the relocations will not exist
> > anymore in the final DSO. As there is no extension point for the
> > compilation of each object file, perform the validation in vdso_check.
> > 
> > Debug information can contain legal absolute relocations and readelf can
> > not print relocations from the .text section only. Make use of the fact
> > that all global vDSO symbols follow the naming pattern "vdso_u_".
> > 
> > Link: https://lore.kernel.org/lkml/aApGPAoctq_eoE2g@t14ultra/
> > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120002
> > Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> > ---
> >   lib/vdso/Makefile.include | 6 ++++++
> >   1 file changed, 6 insertions(+)
> > 
> > diff --git a/lib/vdso/Makefile.include b/lib/vdso/Makefile.include
> > index cedbf15f80874d4bb27c097244bc5b11272f261c..04257d0f28c0ed324e31adbb68497181085752f8 100644
> > --- a/lib/vdso/Makefile.include
> > +++ b/lib/vdso/Makefile.include
> > @@ -12,7 +12,13 @@ c-getrandom-$(CONFIG_VDSO_GETRANDOM) := $(addprefix $(GENERIC_VDSO_DIR), getrand
> >   #
> >   # As a workaround for some GNU ld ports which produce unneeded R_*_NONE
> >   # dynamic relocations, ignore R_*_NONE.
> > +#
> > +# Also validate that no absolute relocations against global symbols are present
> > +# in the object files.
> >   quiet_cmd_vdso_check = VDSOCHK $@
> >         cmd_vdso_check = if $(READELF) -rW $@ | grep -v _NONE | grep -q " R_\w*_"; \
> >   		       then (echo >&2 "$@: dynamic relocations are not supported"; \
> > +			     rm -f $@; /bin/false); fi && \
> > +		       if $(READELF) -rW $(filter %.o, $(real-prereqs)) | grep -q " R_\w*_ABS.*vdso_u_"; \
> 
> 
> This only works for arm64 relocations right? I can't find any *ABS*
> relocations in riscv elf abi
> (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/releases/tag/v1.0).

Indeed you are right.

We could introduce per-architecture configuration. Essentially reverting parts
of commit aff69273af61 ("vdso: Improve cmd_vdso_check to check all dynamic relocations").
The final logic for the intermediary objects still needs to be more complicated
than for the final .so as those contain relocations in the debug information.

Or we could add a C hostprog for validation.
That would be much more flexible than the inline shell command.
It would then also be easier to use an allow-list than the brittle deny-list.

Or we don't do anything, relying on the selftests to detect miscompilations.

I'll run this by tglx. If somebody else has any opinions, I'm all ears.

> > +		       then (echo >&2 "$@: absolute relocations are not supported"; \
> >   			     rm -f $@; /bin/false); fi
> > 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/3] vdso: Reject absolute relocations during build
  2025-06-12  8:31   ` Alexandre Ghiti
  2025-06-12 14:21     ` Thomas Weißschuh
@ 2025-06-12 20:05     ` Palmer Dabbelt
  2025-06-13  1:08       ` Jessica Clarke
  1 sibling, 1 reply; 15+ messages in thread
From: Palmer Dabbelt @ 2025-06-12 20:05 UTC (permalink / raw)
  To: Alexandre Ghiti
  Cc: thomas.weissschuh, Paul Walmsley, aou, nathan,
	nick.desaulniers+lkml, morbo, justinstitt, luto, tglx,
	vincenzo.frascino, linux-riscv, linux-kernel, llvm

On Thu, 12 Jun 2025 01:31:20 PDT (-0700), Alexandre Ghiti wrote:
> Hi Thomas,
>
> On 6/11/25 11:22, Thomas Weißschuh wrote:
>> All vDSO code needs to be completely position independent.
>> Symbol references are marked as hidden so the compiler emits
>> PC-relative relocations. However there are cases where the compiler may
>> still emit absolute relocations, as they are valid in regular PIC DSO code.
>> These would be resolved by the linker and will break at runtime.
>> This has been observed on arm64 under some circumstances, see
>> commit 0c314cda9325 ("arm64: vdso: Work around invalid absolute relocations from GCC")
>>
>> Introduce a build-time check for absolute relocations.
>> The check is done on the object files as the relocations will not exist
>> anymore in the final DSO. As there is no extension point for the
>> compilation of each object file, perform the validation in vdso_check.
>>
>> Debug information can contain legal absolute relocations and readelf can
>> not print relocations from the .text section only. Make use of the fact
>> that all global vDSO symbols follow the naming pattern "vdso_u_".
>>
>> Link: https://lore.kernel.org/lkml/aApGPAoctq_eoE2g@t14ultra/
>> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120002
>> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
>> ---
>>   lib/vdso/Makefile.include | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/lib/vdso/Makefile.include b/lib/vdso/Makefile.include
>> index cedbf15f80874d4bb27c097244bc5b11272f261c..04257d0f28c0ed324e31adbb68497181085752f8 100644
>> --- a/lib/vdso/Makefile.include
>> +++ b/lib/vdso/Makefile.include
>> @@ -12,7 +12,13 @@ c-getrandom-$(CONFIG_VDSO_GETRANDOM) := $(addprefix $(GENERIC_VDSO_DIR), getrand
>>   #
>>   # As a workaround for some GNU ld ports which produce unneeded R_*_NONE
>>   # dynamic relocations, ignore R_*_NONE.
>> +#
>> +# Also validate that no absolute relocations against global symbols are present
>> +# in the object files.
>>   quiet_cmd_vdso_check = VDSOCHK $@
>>         cmd_vdso_check = if $(READELF) -rW $@ | grep -v _NONE | grep -q " R_\w*_"; \
>>   		       then (echo >&2 "$@: dynamic relocations are not supported"; \
>> +			     rm -f $@; /bin/false); fi && \
>> +		       if $(READELF) -rW $(filter %.o, $(real-prereqs)) | grep -q " R_\w*_ABS.*vdso_u_"; \
>
>
> This only works for arm64 relocations right? I can't find any *ABS*
> relocations in riscv elf abi
> (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/releases/tag/v1.0).

That's because the psABI people do not believe in absolute symbols.  
They exist in the actual toolchains, though, as they are part of the 
generic ELF ABI.  In theory they'd work fine in the VDSO as long as 
we're using absolute addressing instructions for them, which is 
possible to do (and I think should happen for some global symbols).

That said, it doesn't really seem worth the effort to get the checking 
more fine-grained here.  I don't see any reason we'd need an absolute 
symbol in the VDSO, so unil someone has one we might as well just forbid 
them entirely.

Some old toolchains had an absolute "__gloabl_pointer$" floating around 
some of the CRT files, so we might trip over bugs here.  I think we're 
safe as those shouldn't show up in the VDSO, but not 100% sure.  
Probably best to get this on next to bake for a bit, just to make sure 
we're not trippig anyone up.

> Thanks,
>
> Alex
>
>
>> +		       then (echo >&2 "$@: absolute relocations are not supported"; \
>>   			     rm -f $@; /bin/false); fi
>>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/3] vdso: Reject absolute relocations during build
  2025-06-12 20:05     ` Palmer Dabbelt
@ 2025-06-13  1:08       ` Jessica Clarke
  0 siblings, 0 replies; 15+ messages in thread
From: Jessica Clarke @ 2025-06-13  1:08 UTC (permalink / raw)
  To: Palmer Dabbelt
  Cc: Alexandre Ghiti, thomas.weissschuh, Paul Walmsley, aou, nathan,
	nick.desaulniers+lkml, morbo, justinstitt, luto, tglx,
	vincenzo.frascino, linux-riscv, linux-kernel, llvm

On 12 Jun 2025, at 21:05, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> 
> On Thu, 12 Jun 2025 01:31:20 PDT (-0700), Alexandre Ghiti wrote:
>> Hi Thomas,
>> 
>> On 6/11/25 11:22, Thomas Weißschuh wrote:
>>> All vDSO code needs to be completely position independent.
>>> Symbol references are marked as hidden so the compiler emits
>>> PC-relative relocations. However there are cases where the compiler may
>>> still emit absolute relocations, as they are valid in regular PIC DSO code.
>>> These would be resolved by the linker and will break at runtime.
>>> This has been observed on arm64 under some circumstances, see
>>> commit 0c314cda9325 ("arm64: vdso: Work around invalid absolute relocations from GCC")
>>> 
>>> Introduce a build-time check for absolute relocations.
>>> The check is done on the object files as the relocations will not exist
>>> anymore in the final DSO. As there is no extension point for the
>>> compilation of each object file, perform the validation in vdso_check.
>>> 
>>> Debug information can contain legal absolute relocations and readelf can
>>> not print relocations from the .text section only. Make use of the fact
>>> that all global vDSO symbols follow the naming pattern "vdso_u_".
>>> 
>>> Link: https://lore.kernel.org/lkml/aApGPAoctq_eoE2g@t14ultra/
>>> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=120002
>>> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
>>> ---
>>>  lib/vdso/Makefile.include | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>> 
>>> diff --git a/lib/vdso/Makefile.include b/lib/vdso/Makefile.include
>>> index cedbf15f80874d4bb27c097244bc5b11272f261c..04257d0f28c0ed324e31adbb68497181085752f8 100644
>>> --- a/lib/vdso/Makefile.include
>>> +++ b/lib/vdso/Makefile.include
>>> @@ -12,7 +12,13 @@ c-getrandom-$(CONFIG_VDSO_GETRANDOM) := $(addprefix $(GENERIC_VDSO_DIR), getrand
>>>  #
>>>  # As a workaround for some GNU ld ports which produce unneeded R_*_NONE
>>>  # dynamic relocations, ignore R_*_NONE.
>>> +#
>>> +# Also validate that no absolute relocations against global symbols are present
>>> +# in the object files.
>>>  quiet_cmd_vdso_check = VDSOCHK $@
>>>        cmd_vdso_check = if $(READELF) -rW $@ | grep -v _NONE | grep -q " R_\w*_"; \
>>>          then (echo >&2 "$@: dynamic relocations are not supported"; \
>>> +      rm -f $@; /bin/false); fi && \
>>> +        if $(READELF) -rW $(filter %.o, $(real-prereqs)) | grep -q " R_\w*_ABS.*vdso_u_"; \
>> 
>> 
>> This only works for arm64 relocations right? I can't find any *ABS*
>> relocations in riscv elf abi
>> (https://github.com/riscv-non-isa/riscv-elf-psabi-doc/releases/tag/v1.0).
> 
> That's because the psABI people do not believe in absolute symbols.

I don’t know what you’re referring to here. Absolute symbols (SHN_ABS
rather than a real section index) are different to absolute
relocations. The code here is checking for *relocations*, not symbols.
RISC-V has absolute relocations, they’re R_RISCV_32/64. Putting ABS in
the relocation name is an Arm-ism that other architectures don’t do.
Absolute addressing exists, as the medlow and large code models (with
-fno-pic/PIC/pie/PIE). So I’m really not sure what you’re saying the
current psABI doesn’t do that it should, nor what discussions you’re
even suggesting have happened and been dismissed?

Perhaps you’re referring to the difficulty of using PC-relative
accesses against absolute symbols, as you’ll get in medany and PIE
code? But that’s something that affects many architectures; you can
have issues on amd64 and aarch64, not just riscv64.

Jessica

> They exist in the actual toolchains, though, as they are part of the generic ELF ABI. In theory they'd work fine in the VDSO as long as we're using absolute addressing instructions for them, which is possible to do (and I think should happen for some global symbols).
> 
> That said, it doesn't really seem worth the effort to get the checking more fine-grained here.  I don't see any reason we'd need an absolute symbol in the VDSO, so unil someone has one we might as well just forbid them entirely.
> 
> Some old toolchains had an absolute "__gloabl_pointer$" floating around some of the CRT files, so we might trip over bugs here.  I think we're safe as those shouldn't show up in the VDSO, but not 100% sure.  Probably best to get this on next to bake for a bit, just to make sure we're not trippig anyone up.
> 
>> Thanks,
>> 
>> Alex
>> 
>> 
>>> +        then (echo >&2 "$@: absolute relocations are not supported"; \
>>>        rm -f $@; /bin/false); fi
>>> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/3] vdso: Reject absolute relocations during build
  2025-06-12 14:21     ` Thomas Weißschuh
@ 2025-06-21 15:42       ` Thomas Gleixner
  2025-06-23  7:25         ` Thomas Weißschuh
  0 siblings, 1 reply; 15+ messages in thread
From: Thomas Gleixner @ 2025-06-21 15:42 UTC (permalink / raw)
  To: Thomas Weißschuh, Alexandre Ghiti
  Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, Nathan Chancellor,
	Nick Desaulniers, Bill Wendling, Justin Stitt, Andy Lutomirski,
	Vincenzo Frascino, linux-riscv, linux-kernel, llvm

On Thu, Jun 12 2025 at 16:21, Thomas Weißschuh wrote:
> On Thu, Jun 12, 2025 at 10:31:20AM +0200, Alexandre Ghiti wrote:
> We could introduce per-architecture configuration. Essentially reverting parts
> of commit aff69273af61 ("vdso: Improve cmd_vdso_check to check all dynamic relocations").
> The final logic for the intermediary objects still needs to be more complicated
> than for the final .so as those contain relocations in the debug information.
>
> Or we could add a C hostprog for validation.
> That would be much more flexible than the inline shell command.
> It would then also be easier to use an allow-list than the brittle deny-list.
>
> Or we don't do anything, relying on the selftests to detect miscompilations.

That's a bad idea :)

> I'll run this by tglx. If somebody else has any opinions, I'm all ears.

This is all a mess because the relocation type numbers and their R_*
names are not uniform accross architectures. Neither are the valid
relocation types which are suitable for VDSO.

I don't think you can reasonably cover all of it with readelf and
grep. I did some unrelated relocation analysis some time ago and I just
modified the python script (yes, I hate to use libelf) to show case how
insane this gets. This is just as much as I needed to analyse files
compiled with some random cross gcc I had handy. But you surely get the
idea.

Thanks,

        tglx
---
#!/usr/bin/env python3

import sys

from argparse import ArgumentParser
from elftools.elf.elffile import ELFFile
from elftools.elf.relocation import RelocationSection
from elftools.elf.enums import ENUM_RELOC_TYPE_i386, ENUM_RELOC_TYPE_x64
from elftools.elf.enums import ENUM_RELOC_TYPE_ARM, ENUM_RELOC_TYPE_AARCH64
from elftools.elf.descriptions import describe_reloc_type

class relocs(object):
    def __init__(self, arch, sections, types):
        self.arch = arch
        self.sections = sections
        self.types = types

i386_relocs = relocs('EM_386',
                     [ '.rel.text' ],
                     [ ENUM_RELOC_TYPE_i386['R_386_NONE'],
                       ENUM_RELOC_TYPE_i386['R_386_PC32'],
                       ENUM_RELOC_TYPE_i386['R_386_GOTPC'],
                       ENUM_RELOC_TYPE_i386['R_386_GOTOFF'],
                      ])

x86_64_relocs = relocs('EM_X86_64',
                       [ '.rela.text' ],
                       [ ENUM_RELOC_TYPE_x64['R_X86_64_NONE'],
                         ENUM_RELOC_TYPE_x64['R_X86_64_PC32'],
                        ])

arm_relocs = relocs('EM_ARM',
                       [ '.rela.text' ],
                       # Probably incomplete
                       [ ENUM_RELOC_TYPE_ARM['R_ARM_NONE'],
                         ENUM_RELOC_TYPE_ARM['R_ARM_REL32'],
                        ])

arm64_relocs = relocs('EM_AARCH64',
                       [ '.rela.text' ],
                       # Probably incomplete
                       [ ENUM_RELOC_TYPE_AARCH64['R_AARCH64_NONE'],
                         ENUM_RELOC_TYPE_AARCH64['R_AARCH64_ADR_PREL_LO21'],
                        ])

# Minimal set for an example VDSO build
ENUM_RELOC_TYPE_RISCV = dict(
    R_RISCV_BRANCH        = 0x10,
    R_RISCV_PCREL_HI20    = 0x17,
    R_RISCV_PCREL_LO12_I  = 0x18,
    R_RISCV_RVC_BRANCH    = 0x2c,
    R_RISCV_RVC_JUMP      = 0x2d,
    R_RISCV_RELAX         = 0x33,
)

riscv_relocs = relocs('EM_RISCV',
                       [ '.rela.text' ],
                       [ ENUM_RELOC_TYPE_RISCV['R_RISCV_BRANCH'],
                         ENUM_RELOC_TYPE_RISCV['R_RISCV_PCREL_HI20'],
                         ENUM_RELOC_TYPE_RISCV['R_RISCV_PCREL_LO12_I'],
                         ENUM_RELOC_TYPE_RISCV['R_RISCV_RVC_BRANCH'],
                         ENUM_RELOC_TYPE_RISCV['R_RISCV_RVC_JUMP'],
                         ENUM_RELOC_TYPE_RISCV['R_RISCV_RELAX'],
                        ])
supported_archs = {
    'i386'      : i386_relocs,
    'x86_64'    : x86_64_relocs,
    'arm'       : arm_relocs,
    'arm64'     : arm64_relocs,
    'riscv'     : riscv_relocs,
}

# Probably incomplete
invalid_relocs = [ '.rela.dyn', '.rela.plt' ]

def check_relocations(file, arch):
    elf = ELFFile(file)
    res = 0

    if elf.header['e_machine'] != arch.arch:
        print(elf.header['e_machine'], arch.arch)
        raise Exception('Architecture mismatch')

    for section in elf.iter_sections():
        if not isinstance(section, RelocationSection):
            continue

        if section.name in invalid_relocs:
            print('Invalid VDSO relocation section: %s' %section.name)
            res += 1
            continue

        if section.name not in arch.sections:
            continue

        for reloc in section.iter_relocations():
            if reloc['r_info_type'] in arch.types:
                continue
            res += 1

            symt = elf.get_section(section['sh_link'])
            sym = symt.get_symbol(reloc['r_info_sym'])

            type = describe_reloc_type(reloc['r_info_type'], elf)

            print("Invalid VDSO relocation: %s %s" %(type, sym.name))

    return res

if __name__ == '__main__':
    parser = ArgumentParser(usage = 'usage: %(prog)s arch elf-file',
                            description = 'magic VDSO section checker',
                            prog = 'vdsoreloc')

    parser.add_argument('arch',
                        choices = supported_archs.keys(),
                        help = 'Target architecture')
    parser.add_argument('file', help = 'ELF file to parse')
    args = parser.parse_args()

    with open(args.file, 'rb') as file:
        try:
            res = check_relocations(file, supported_archs[args.arch])
            sys.exit(res)
        except Exception as ex:
            # Do something sensible here
            print(ex)
            sys.exit(1)



^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v3 3/3] vdso: Reject absolute relocations during build
  2025-06-21 15:42       ` Thomas Gleixner
@ 2025-06-23  7:25         ` Thomas Weißschuh
  0 siblings, 0 replies; 15+ messages in thread
From: Thomas Weißschuh @ 2025-06-23  7:25 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Alexandre Ghiti, Paul Walmsley, Palmer Dabbelt, Albert Ou,
	Nathan Chancellor, Nick Desaulniers, Bill Wendling, Justin Stitt,
	Andy Lutomirski, Vincenzo Frascino, linux-riscv, linux-kernel,
	llvm

On Sat, Jun 21, 2025 at 05:42:20PM +0200, Thomas Gleixner wrote:
> On Thu, Jun 12 2025 at 16:21, Thomas Weißschuh wrote:
> > On Thu, Jun 12, 2025 at 10:31:20AM +0200, Alexandre Ghiti wrote:
> > We could introduce per-architecture configuration. Essentially reverting parts
> > of commit aff69273af61 ("vdso: Improve cmd_vdso_check to check all dynamic relocations").
> > The final logic for the intermediary objects still needs to be more complicated
> > than for the final .so as those contain relocations in the debug information.
> >
> > Or we could add a C hostprog for validation.
> > That would be much more flexible than the inline shell command.
> > It would then also be easier to use an allow-list than the brittle deny-list.
> >
> > Or we don't do anything, relying on the selftests to detect miscompilations.
> 
> That's a bad idea :)

Fully agreed :-)

> > I'll run this by tglx. If somebody else has any opinions, I'm all ears.
> 
> This is all a mess because the relocation type numbers and their R_*
> names are not uniform accross architectures. Neither are the valid
> relocation types which are suitable for VDSO.

Ack.

> I don't think you can reasonably cover all of it with readelf and
> grep. I did some unrelated relocation analysis some time ago and I just
> modified the python script (yes, I hate to use libelf) to show case how
> insane this gets. This is just as much as I needed to analyse files
> compiled with some random cross gcc I had handy. But you surely get the
> idea.

Yes I get the idea. This is more or less exactly what I meant above with:
"Or we could add a C hostprog for validation."
More specifically my plan then is to write a C application that uses
<linux/elf.h> to do what your Python script does.
There should be no need to mess with libelf.

<snip>


Thomas

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2025-06-23  7:25 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-11  9:22 [PATCH v3 0/3] vdso: Reject absolute relocations during build Thomas Weißschuh
2025-06-11  9:22 ` [PATCH v3 1/3] riscv: vdso: Deduplicate CFLAGS_REMOVE_* variables Thomas Weißschuh
2025-06-11 18:40   ` Nathan Chancellor
2025-06-12  8:13     ` Alexandre Ghiti
2025-06-11  9:22 ` [PATCH v3 2/3] riscv: vdso: Disable LTO for the vDSO Thomas Weißschuh
2025-06-11 18:41   ` Nathan Chancellor
2025-06-11  9:22 ` [PATCH v3 3/3] vdso: Reject absolute relocations during build Thomas Weißschuh
2025-06-11 18:57   ` Nathan Chancellor
2025-06-12  5:48     ` Thomas Weißschuh
2025-06-12  8:31   ` Alexandre Ghiti
2025-06-12 14:21     ` Thomas Weißschuh
2025-06-21 15:42       ` Thomas Gleixner
2025-06-23  7:25         ` Thomas Weißschuh
2025-06-12 20:05     ` Palmer Dabbelt
2025-06-13  1:08       ` Jessica Clarke

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).