linux-kernel - Re: [PATCH 6/7] x86: objtool: add support for R_X86_64_REX

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: <CAMj1kXEARA1KnD95RO=huLeQ-8nLsGixg0nOx01k4jgkb-2GYQ@mail.gmail.com>
Date: Thu, 17 Apr 2025 17:49:13 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: Alexander Potapenko <glider@...gle.com>
Cc: Uros Bizjak <ubizjak@...il.com>, quic_jiangenj@...cinc.com, 
	linux-kernel@...r.kernel.org, kasan-dev@...glegroups.com, x86@...nel.org, 
	Aleksandr Nogikh <nogikh@...gle.com>, Andrey Konovalov <andreyknvl@...il.com>, Borislav Petkov <bp@...en8.de>, 
	Dave Hansen <dave.hansen@...ux.intel.com>, Dmitry Vyukov <dvyukov@...gle.com>, 
	Ingo Molnar <mingo@...hat.com>, Josh Poimboeuf <jpoimboe@...nel.org>, Marco Elver <elver@...gle.com>, 
	Peter Zijlstra <peterz@...radead.org>, Thomas Gleixner <tglx@...utronix.de>
Subject: Re: [PATCH 6/7] x86: objtool: add support for R_X86_64_REX_GOTPCRELX

Hi,

On Thu, 17 Apr 2025 at 17:37, Alexander Potapenko <glider@...gle.com> wrote:
>
> On Wed, Apr 16, 2025 at 4:21 PM Uros Bizjak <ubizjak@...il.com> wrote:
> >
> >
> >
> > On 16. 04. 25 10:54, Alexander Potapenko wrote:
> > > When compiling modules with -fsanitize-coverage=trace-pc-guard, Clang
> > > will emit R_X86_64_REX_GOTPCRELX relocations for the
> > > __start___sancov_guards and __stop___sancov_guards symbols. Although
> > > these relocations can be resolved within the same binary, they are left
> > > over by the linker because of the --emit-relocs flag.
> > >

Not sure what you mean here - --emit-relocs is not used for modules,
only for vmlinux.

> > > This patch makes it possible to resolve the R_X86_64_REX_GOTPCRELX
> > > relocations at runtime, as doing so does not require a .got section.

Why not? R_X86_64_REX_GOTPCRELX is *not* a PC32 reference to the
symbol, it is a PC32 reference to a 64-bit global variable that
contains the absolute address of the symbol.

> > > In addition, add a missing overflow check to R_X86_64_PC32/R_X86_64_PLT32.
> > >
> > > Cc: x86@...nel.org
> > > Signed-off-by: Alexander Potapenko <glider@...gle.com>
> > > ---
> > >   arch/x86/include/asm/elf.h      | 1 +
> > >   arch/x86/kernel/module.c        | 8 ++++++++
> > >   arch/x86/um/asm/elf.h           | 1 +
> > >   tools/objtool/arch/x86/decode.c | 1 +
> > >   4 files changed, 11 insertions(+)
> > >
> > > diff --git a/arch/x86/include/asm/elf.h b/arch/x86/include/asm/elf.h
> > > index 1fb83d47711f9..15d0438467e94 100644
> > > --- a/arch/x86/include/asm/elf.h
> > > +++ b/arch/x86/include/asm/elf.h
> > > @@ -63,6 +63,7 @@ typedef struct user_i387_struct elf_fpregset_t;
> > >   #define R_X86_64_8          14      /* Direct 8 bit sign extended  */
> > >   #define R_X86_64_PC8                15      /* 8 bit sign extended pc relative */
> > >   #define R_X86_64_PC64               24      /* Place relative 64-bit signed */
> > > +#define R_X86_64_REX_GOTPCRELX       42      /* R_X86_64_GOTPCREL with optimizations */
> > >

Why do you need this? arch/x86/kernel/module.c already has a reference
to R_X86_64_REX_GOTPCRELX so surely it is defined already somewhere.

> > >   /*
> > >    * These are used to set parameters in the core dumps.
> > > diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
> > > index 8984abd91c001..6c8b524bfbe3b 100644
> > > --- a/arch/x86/kernel/module.c
> > > +++ b/arch/x86/kernel/module.c
> > > @@ -133,6 +133,14 @@ static int __write_relocate_add(Elf64_Shdr *sechdrs,
> > >               case R_X86_64_PC32:
> > >               case R_X86_64_PLT32:
> > >                       val -= (u64)loc;
> > > +                     if ((s64)val != *(s32 *)&val)
> > > +                             goto overflow;
> > > +                     size = 4;
> > > +                     break;
> > > +             case R_X86_64_REX_GOTPCRELX:
> > > +                     val -= (u64)loc;
> > > +                     if ((s64)val != *(s32 *)&val)
> > > +                             goto overflow;
> > >                       size = 4;
> > >                       break;
> >
> > These two cases are the same. You probably want:
> >
> >                 case R_X86_64_PC32:
> >                 case R_X86_64_PLT32:
> >                 case R_X86_64_REX_GOTPCRELX:
> >                         val -= (u64)loc;
> >                         if ((s64)val != *(s32 *)&val)
> >                                 goto overflow;
> >                         size = 4;
> >                         break;
> >
>
> You are right, I overlooked this, as well as the other
> R_X86_64_REX_GOTPCRELX case above.

They are most definitely *not* the same.

> Ard, do you think we can relax the code handling __stack_chk_guard to
> accept every R_X86_64_REX_GOTPCRELX relocation?

Isn't it possible to discourage Clang from using
R_X86_64_REX_GOTPCRELX? Does -fdirect-access-external-data make any
difference here?

In any case, to resolve these relocations correctly, the reference
need to be made to point to global variables that carry the absolute
addresses of __start___sancov_guards and __stop___sancov_guards.
Ideally, these variables would be allocated and populated on the fly,
similar to how the linker allocates GOT entries at build time. But
this adds a lot of complexity for something that we don't want to see
in the first place.

Alternatively, the references could be relaxed, i.e., MOV converted to
LEA etc. The x86 ELF psABI describes how this is supposed to work for
R_X86_64_REX_GOTPCRELX but it involves rewriting the instructions so
it is a bit tedious.

But it would be much better to just fix the compiler.