Re: [PATCH -tip v2 2/3] x86/callthunks: Handle %rip-relative relocations in call thunk template

[Uros Bizjak]

On Fri, Dec 1, 2023 at 4:55 AM Nathan Chancellor <nathan@xxxxxxxxxx> wrote:
>
> Hi Uros,
>
> On Sun, Nov 05, 2023 at 10:34:36PM +0100, Uros Bizjak wrote:
> > Contrary to alternatives, relocations are currently not supported in
> > call thunk templates.  Re-use the existing infrastructure from
> > alternative.c to allow %rip-relative relocations when copying call
> > thunk template from its storage location.
> >
> > The patch allows unification of ASM_INCREMENT_CALL_DEPTH, which already
> > uses PER_CPU_VAR macro, with INCREMENT_CALL_DEPTH, used in call thunk
> > template, which is currently limited to use absolute address.
> >
> > Cc: Thomas Gleixner <tglx@xxxxxxxxxxxxx>
> > Cc: Ingo Molnar <mingo@xxxxxxxxxx>
> > Cc: Borislav Petkov <bp@xxxxxxxxx>
> > Cc: Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>
> > Cc: "H. Peter Anvin" <hpa@xxxxxxxxx>
> > Cc: Peter Zijlstra <peterz@xxxxxxxxxxxxx>
> > Signed-off-by: Uros Bizjak <ubizjak@xxxxxxxxx>
> ...
> > diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c
> > index e9ad518a5003..ef9c04707b3c 100644
> > --- a/arch/x86/kernel/callthunks.c
> > +++ b/arch/x86/kernel/callthunks.c
> ...
> > @@ -291,20 +298,27 @@ void *callthunks_translate_call_dest(void *dest)
> >  static bool is_callthunk(void *addr)
> >  {
> >       unsigned int tmpl_size = SKL_TMPL_SIZE;
> > -     void *tmpl = skl_call_thunk_template;
> > +     u8 insn_buff[MAX_PATCH_LEN];
> >       unsigned long dest;
> > +     u8 *pad;
> >
> >       dest = roundup((unsigned long)addr, CONFIG_FUNCTION_ALIGNMENT);
> >       if (!thunks_initialized || skip_addr((void *)dest))
> >               return false;
> >
> > -     return !bcmp((void *)(dest - tmpl_size), tmpl, tmpl_size);
> > +     *pad = dest - tmpl_size;
>
> Clang warns (or errors with CONFIG_WERROR=y):

Uh, GCC didn't warn at all (and there is some mixup with types here,
so a thinko slipped through.

The attached patch fixes the oversight. I'll post a formal patch later
today after some more testing.

Thanks,
Uros.
diff --git a/arch/x86/kernel/callthunks.c b/arch/x86/kernel/callthunks.c
index f5507c95e7be..71b74a07c8ee 100644
--- a/arch/x86/kernel/callthunks.c
+++ b/arch/x86/kernel/callthunks.c
@@ -306,7 +306,7 @@ static bool is_callthunk(void *addr)
 	if (!thunks_initialized || skip_addr((void *)dest))
 		return false;
 
-	*pad = dest - tmpl_size;
+	pad = (void *)dest - tmpl_size;
 
 	memcpy(insn_buff, skl_call_thunk_template, tmpl_size);
 	apply_relocation(insn_buff, tmpl_size, pad,