linux-kernel - Re: [PATCH v3 3/3] vdso: Reject absolute relocations during build

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250612155635-ecade4e1-0235-464a-bcb3-293f7452510a@linutronix.de>
Date: Thu, 12 Jun 2025 16:21:33 +0200
From: Thomas Weißschuh <thomas.weissschuh@...utronix.de>
To: Alexandre Ghiti <alex@...ti.fr>
Cc: Paul Walmsley <paul.walmsley@...ive.com>, 
	Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>, 
	Nathan Chancellor <nathan@...nel.org>, Nick Desaulniers <nick.desaulniers+lkml@...il.com>, 
	Bill Wendling <morbo@...gle.com>, Justin Stitt <justinstitt@...gle.com>, 
	Andy Lutomirski <luto@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, 
	Vincenzo Frascino <vincenzo.frascino@....com>, linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org, 
	llvm@...ts.linux.dev
Subject: Re: [PATCH v3 3/3] vdso: Reject absolute relocations during build

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@...utronix.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
> >