Messages in this thread | | | Date | Tue, 23 Apr 2024 18:24:05 +0200 | Subject | Re: [PATCH v5] kallsyms: Avoid weak references for kallsyms symbols | From | Konrad Dybcio <> |
| |
On 4/23/24 18:22, Ard Biesheuvel wrote: > On Tue, 23 Apr 2024 at 18:01, Konrad Dybcio <konrad.dybcio@linaro.org> wrote: >> >> >> >> On 4/22/24 18:02, Masahiro Yamada wrote: >>> On Sat, Apr 20, 2024 at 11:53 PM Ard Biesheuvel <ardb+git@google.com> wrote: >>>> >>>> From: Ard Biesheuvel <ardb@kernel.org> >>>> >>>> kallsyms is a directory of all the symbols in the vmlinux binary, and so >>>> creating it poses somewhat of a chicken-and-egg problem, as its non-zero >>>> size affects the layout of the binary, and therefore the values of the >>>> symbols. >>>> >>>> For this reason, the kernel is linked more than once, and the first pass >>>> does not include any kallsyms data at all. For the linker to accept >>>> this, the symbol declarations describing the kallsyms metadata are >>>> emitted as having weak linkage, so they can remain unsatisfied. During >>>> the subsequent passes, the weak references are satisfied by the kallsyms >>>> metadata that was constructed based on information gathered from the >>>> preceding passes. >>>> >>>> Weak references lead to somewhat worse codegen, because taking their >>>> address may need to produce NULL (if the reference was unsatisfied), and >>>> this is not usually supported by RIP or PC relative symbol references. >>>> >>>> Given that these references are ultimately always satisfied in the final >>>> link, let's drop the weak annotation on the declarations, and instead, >>>> provide fallback definitions with weak linkage. This informs the >>>> compiler that ultimately, the reference will always be satisfied. >>>> >>>> While at it, drop the FRV specific annotation that these symbols reside >>>> in .rodata - FRV is long gone. >>>> >>>> Cc: Masahiro Yamada <masahiroy@kernel.org> >>>> Cc: linux-kbuild@vger.kernel.org >>>> Acked-by: Nick Desaulniers <ndesaulniers@google.com> >>>> Acked-by: Kees Cook <keescook@chromium.org> >>>> Acked-by: Arnd Bergmann <arnd@arndb.de> >>>> Link: https://lore.kernel.org/all/20240415075837.2349766-5-ardb+git@google.com >>>> Signed-off-by: Ard Biesheuvel <ardb@kernel.org> >>>> --- >>>> v5: - avoid PROVIDE() in the linker script, use weak definitions instead >>>> - drop tested-by, replace reviewed-by with acked-by >>>> >>> >>> Applied to linux-kbuild. Thanks. >> >> Hi, this commit seems to break call traces, resulting in output like: >> >> [ 2.777006] Call trace: >> [ 2.777007] _text+0x89e7e8/0x39e0000 >> [ 2.777008] _text+0x89e82c/0x39e0000 >> [ 2.777009] _text+0x2b940cc/0x2bd2a90 >> [ 2.777011] _text+0x2b941a4/0x2bd2a90 >> [ 2.777012] _text+0x145dc/0x39e0000 >> [ 2.777014] _text+0x2b51184/0x2bd2a90 >> [ 2.777016] _text+0x18fc6a4/0x39e0000 >> [ 2.777018] _text+0x15644/0x39e0000 >> [ 2.777019] ---[ end trace 0000000000000000 ]--- >> > > This patch triggers an issue in the compiler, which appears to perform > constant propagation on variables defined as weak, and this is > definitely a compiler bug. (A weak variable can be superseded by > another instance from a different object at link time, so the compiler > cannot make assumptions based on the version of the variable it > observes at compile time)
Sounds like fun..
> > It has already been dropped from the kbuild tree.
Thanks!
Konrad
| |