[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <mhng-4BA05CCD-6D5A-4067-B88A-DEBD4FCDED77@palmerdabbelt-mac>
Date: Thu, 12 Jun 2025 13:05:55 -0700 (PDT)
From: Palmer Dabbelt <palmer@...belt.com>
To: Alexandre Ghiti <alex@...ti.fr>
CC: thomas.weissschuh@...utronix.de, Paul Walmsley <paul.walmsley@...ive.com>,
aou@...s.berkeley.edu, nathan@...nel.org, nick.desaulniers+lkml@...il.com, morbo@...gle.com,
justinstitt@...gle.com, luto@...nel.org, tglx@...utronix.de, 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
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@...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).
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
>>
Powered by blists - more mailing lists