lkdtm: Annotate do_nothing()

lkdtm: Annotate do_nothing()

[Peter Zijlstra]

Hi Kees,

During my FineIBT testing the other week I stumbled upon the following
complaint:

  vmlinux.o: warning: objtool: execute_location+0x4f: relocation to !ENDBR: .text+0x1032008

I finally got around to looking at it and realized we have means of
annotating that since 93f16a1ab78c ("x86/boot: Mark start_secondary() with __noendbr")
(which might still be in tip only).

Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
---
diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
index 5b861dbff27e..9600af4494d8 100644
--- a/drivers/misc/lkdtm/perms.c
+++ b/drivers/misc/lkdtm/perms.c
@@ -36,6 +36,7 @@ static noinline void do_nothing(void)
 {
 	return;
 }
+ANNOTATE_NOENDBR_SYM(do_nothing);
 
 /* Must immediately follow do_nothing for size calculuations to work out. */
 static noinline void do_overwritten(void)

Re: lkdtm: Annotate do_nothing()

[Kees Cook]

On Mon, Mar 03, 2025 at 10:38:17AM +0100, Peter Zijlstra wrote:
> Hi Kees,
> 
> During my FineIBT testing the other week I stumbled upon the following
> complaint:
> 
>   vmlinux.o: warning: objtool: execute_location+0x4f: relocation to !ENDBR: .text+0x1032008
> 
> I finally got around to looking at it and realized we have means of
> annotating that since 93f16a1ab78c ("x86/boot: Mark start_secondary() with __noendbr")
> (which might still be in tip only).

Er, doesn't that mean do_nothing() will lack an ENDBR? Wait, no, that's
__noendbr. What does this annotation mean if the function _does_ have
ENDBR?

Note that these tests are explicitly using __nocfi (via the
execute_location() function) since they're testing the Execute bit in
different memory regions. But I would expect BTI to still work (and not
block execution).

-Kees

> 
> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
> ---
> diff --git a/drivers/misc/lkdtm/perms.c b/drivers/misc/lkdtm/perms.c
> index 5b861dbff27e..9600af4494d8 100644
> --- a/drivers/misc/lkdtm/perms.c
> +++ b/drivers/misc/lkdtm/perms.c
> @@ -36,6 +36,7 @@ static noinline void do_nothing(void)
>  {
>  	return;
>  }
> +ANNOTATE_NOENDBR_SYM(do_nothing);
>  
>  /* Must immediately follow do_nothing for size calculuations to work out. */
>  static noinline void do_overwritten(void)

-- 
Kees Cook

Re: lkdtm: Annotate do_nothing()

[Peter Zijlstra]

On Mon, Mar 03, 2025 at 09:26:57AM -0800, Kees Cook wrote:
> On Mon, Mar 03, 2025 at 10:38:17AM +0100, Peter Zijlstra wrote:
> > Hi Kees,
> > 
> > During my FineIBT testing the other week I stumbled upon the following
> > complaint:
> > 
> >   vmlinux.o: warning: objtool: execute_location+0x4f: relocation to !ENDBR: .text+0x1032008
> > 
> > I finally got around to looking at it and realized we have means of
> > annotating that since 93f16a1ab78c ("x86/boot: Mark start_secondary() with __noendbr")
> > (which might still be in tip only).
> 
> Er, doesn't that mean do_nothing() will lack an ENDBR? Wait, no, that's
> __noendbr. What does this annotation mean if the function _does_ have
> ENDBR?
> 
> Note that these tests are explicitly using __nocfi (via the
> execute_location() function) since they're testing the Execute bit in
> different memory regions. But I would expect BTI to still work (and not
> block execution).
> 

Argh, I had a definite wake-up juice deficit this morning, and the BTF
noise made me miss that the warning wasn't fixed.

For some reason I thought do_nothing() didn't have ENDBR. Looking at the
build now, I see it does have, and the actual location pointed to is
do_nothing+0x18, which is weird.

Ooh, I see, the thing looks to have unrolled and inlined that memcpy()
like:

   156a3:       48 8b 05 00 00 00 00    mov    0x0(%rip),%rax        # 156aa <execute_location+0x2a>    156a6: R_X86_64_PC32    .text+0x1032024
   156aa:       48 89 43 38             mov    %rax,0x38(%rbx)
   156ae:       48 8b 05 00 00 00 00    mov    0x0(%rip),%rax        # 156b5 <execute_location+0x35>    156b1: R_X86_64_PC32    .text+0x103201c
   156b5:       48 89 43 30             mov    %rax,0x30(%rbx)
   156b9:       48 8b 05 00 00 00 00    mov    0x0(%rip),%rax        # 156c0 <execute_location+0x40>    156bc: R_X86_64_PC32    .text+0x1032014
   156c0:       48 89 43 28             mov    %rax,0x28(%rbx)
   156c4:       48 8b 05 00 00 00 00    mov    0x0(%rip),%rax        # 156cb <execute_location+0x4b>    156c7: R_X86_64_PC32    .text+0x103200c
   156cb:       48 89 43 20             mov    %rax,0x20(%rbx)
   156cf:       48 8b 05 00 00 00 00    mov    0x0(%rip),%rax        # 156d6 <execute_location+0x56>    156d2: R_X86_64_PC32    .text+0x1032004
   156d6:       48 89 43 18             mov    %rax,0x18(%rbx)
   156da:       48 8b 05 00 00 00 00    mov    0x0(%rip),%rax        # 156e1 <execute_location+0x61>    156dd: R_X86_64_PC32    .text+0x1031ffc
   156e1:       48 89 43 10             mov    %rax,0x10(%rbx)
   156e5:       48 8b 05 00 00 00 00    mov    0x0(%rip),%rax        # 156ec <execute_location+0x6c>    156e8: R_X86_64_PC32    .text+0x1031ff4
   156ec:       48 89 43 08             mov    %rax,0x8(%rbx)
   156f0:       48 8b 05 00 00 00 00    mov    0x0(%rip),%rax        # 156f7 <execute_location+0x77>    156f3: R_X86_64_PC32    .text+0x1031fec
   156f7:       48 89 03                mov    %rax,(%rbx)

And objtool figures those .text references are an address-taken-of like
thing and expects ENDBR at them.

Lovely stuff...

Anyway, ignore this patch.

Re: lkdtm: Annotate do_nothing()

[Kees Cook]

On Mon, Mar 03, 2025 at 07:35:25PM +0100, Peter Zijlstra wrote:
> On Mon, Mar 03, 2025 at 09:26:57AM -0800, Kees Cook wrote:
> > On Mon, Mar 03, 2025 at 10:38:17AM +0100, Peter Zijlstra wrote:
> > > Hi Kees,
> > > 
> > > During my FineIBT testing the other week I stumbled upon the following
> > > complaint:
> > > 
> > >   vmlinux.o: warning: objtool: execute_location+0x4f: relocation to !ENDBR: .text+0x1032008
> > > 
> > > I finally got around to looking at it and realized we have means of
> > > annotating that since 93f16a1ab78c ("x86/boot: Mark start_secondary() with __noendbr")
> > > (which might still be in tip only).
> > 
> > Er, doesn't that mean do_nothing() will lack an ENDBR? Wait, no, that's
> > __noendbr. What does this annotation mean if the function _does_ have
> > ENDBR?
> > 
> > Note that these tests are explicitly using __nocfi (via the
> > execute_location() function) since they're testing the Execute bit in
> > different memory regions. But I would expect BTI to still work (and not
> > block execution).
> > 
> 
> Argh, I had a definite wake-up juice deficit this morning, and the BTF
> noise made me miss that the warning wasn't fixed.
> 
> For some reason I thought do_nothing() didn't have ENDBR. Looking at the
> build now, I see it does have, and the actual location pointed to is
> do_nothing+0x18, which is weird.
> 
> Ooh, I see, the thing looks to have unrolled and inlined that memcpy()
> like:
> 
>    156a3:       48 8b 05 00 00 00 00    mov    0x0(%rip),%rax        # 156aa <execute_location+0x2a>    156a6: R_X86_64_PC32    .text+0x1032024
>    156aa:       48 89 43 38             mov    %rax,0x38(%rbx)
>    156ae:       48 8b 05 00 00 00 00    mov    0x0(%rip),%rax        # 156b5 <execute_location+0x35>    156b1: R_X86_64_PC32    .text+0x103201c
>    156b5:       48 89 43 30             mov    %rax,0x30(%rbx)
>    156b9:       48 8b 05 00 00 00 00    mov    0x0(%rip),%rax        # 156c0 <execute_location+0x40>    156bc: R_X86_64_PC32    .text+0x1032014
>    156c0:       48 89 43 28             mov    %rax,0x28(%rbx)
>    156c4:       48 8b 05 00 00 00 00    mov    0x0(%rip),%rax        # 156cb <execute_location+0x4b>    156c7: R_X86_64_PC32    .text+0x103200c
>    156cb:       48 89 43 20             mov    %rax,0x20(%rbx)
>    156cf:       48 8b 05 00 00 00 00    mov    0x0(%rip),%rax        # 156d6 <execute_location+0x56>    156d2: R_X86_64_PC32    .text+0x1032004
>    156d6:       48 89 43 18             mov    %rax,0x18(%rbx)
>    156da:       48 8b 05 00 00 00 00    mov    0x0(%rip),%rax        # 156e1 <execute_location+0x61>    156dd: R_X86_64_PC32    .text+0x1031ffc
>    156e1:       48 89 43 10             mov    %rax,0x10(%rbx)
>    156e5:       48 8b 05 00 00 00 00    mov    0x0(%rip),%rax        # 156ec <execute_location+0x6c>    156e8: R_X86_64_PC32    .text+0x1031ff4
>    156ec:       48 89 43 08             mov    %rax,0x8(%rbx)
>    156f0:       48 8b 05 00 00 00 00    mov    0x0(%rip),%rax        # 156f7 <execute_location+0x77>    156f3: R_X86_64_PC32    .text+0x1031fec
>    156f7:       48 89 03                mov    %rax,(%rbx)
> 
> And objtool figures those .text references are an address-taken-of like
> thing and expects ENDBR at them.

Ah! It's (quite reasonably) not expecting text reference in memcpy. ;)

> Lovely stuff...
> 
> Anyway, ignore this patch.

Okay! :)

-- 
Kees Cook