Re: [PATCH v4 0/7] x86: Rid .head.text of all abs references

[Ard Biesheuvel]

On Wed, 1 Jan 2025 at 03:43, Nathan Chancellor <nathan@xxxxxxxxxx> wrote:
>
> Hi Ard,
>
> On Tue, Dec 31, 2024 at 08:29:17PM +0100, Ard Biesheuvel wrote:
> > (cc Nathan)
>
> Thanks for the CC.
>
> > On Tue, 31 Dec 2024 at 11:35, Borislav Petkov <bp@xxxxxxxxx> wrote:
> > >
> > > On Tue, Dec 31, 2024 at 11:12:55AM +0100, Ard Biesheuvel wrote:
> > > > I'll look into this asap, i.e., in a couple of days.
> > >
> > > :-P
> > >
> > > Thanks!
> > >
> >
> > I had a quick look, and managed to reproduce it with Clang 14 but not
> > with Clang 18.
> >
> > It looks like UBSAN is emitting some instrumentation here, in spite of
> > the __no_sanitize_undefined annotation (via __head) on
> > pvalidate_4k_page():
> >
> > arch/x86/coco/sev/core.o:
> >
> > 0000000000000a00 <pvalidate_4k_page>:
> >  ...
> >  b72:   40 88 de                mov    %bl,%sil
> >  b75:   48 c7 c7 00 00 00 00    mov    $0x0,%rdi
> >         b78: R_X86_64_32S       .data+0xb0
> >  b7c:   e8 00 00 00 00          callq  b81 <pvalidate_4k_page+0x181>
> >         b7d: R_X86_64_PLT32     __ubsan_handle_load_invalid_value-0x4
> >
> > So as far as this series is concerned, things are working correctly,
> > and an absolute reference to .data is being flagged in code that may
> > execute before the absolute address in question is even mapped.
>
> It appears that this is related to UBSAN_BOOL. This is reproducible with
> just:
>
> $ echo 'CONFIG_AMD_MEM_ENCRYPT=y
> CONFIG_UBSAN=y
> CONFIG_UBSAN_BOOL=y
> # CONFIG_UBSAN_ALIGNMENT is not set
> # CONFIG_UBSAN_BOUNDS is not set
> # CONFIG_UBSAN_DIV_ZERO is not set
> # CONFIG_UBSAN_ENUM is not set
> # CONFIG_UBSAN_SIGNED_WRAP is not set
> # CONFIG_UBSAN_SHIFT is not set
> # CONFIG_UBSAN_TRAP is not set
> # CONFIG_UBSAN_UNREACHABLE is not set' >kernel/configs/repro.config
>
> $ make -skj"$(nproc)" ARCH=x86_64 LLVM=1 mrproper defconfig repro.config vmlinux
> Absolute reference to symbol '.data' not permitted in .head.text
> make[5]: *** [arch/x86/Makefile.postlink:32: vmlinux] Error 1
> ...
>
> Given that this appears in LLVM 14 but not LLVM 15 and newer, I reverse
> bisected the fix in LLVM to [1], which was actually a fix from a report
> from Linus [2]. That seems like a reasonable change to blame, as UBSAN
> is generating this check from the asm() in pvalidate() and after the
> LLVM fix, that check is no longer generated.
>
> It does seem fishy that __no_sanitize_undefined does not prevent the
> generation of that check... Plugging Linus's original reproducer from
> [2] into Compiler Explorer [3], it seems like __no_sanitize_undefined
> does get respected. It is my understanding that inlining functions that
> do not have attributes that disable instrumentation into ones that do is
> supposed to remove the instrumentation, correct? It seems like
> pvalidate() does get inlined into pvalidate_4k_page() but the
> instrumentation remains. Explicitly adding __no_sanitize_undefined to
> pvalidate() hides this for me.
>

Thanks for the analysis.

Should we perhaps just add the below? All the other sanitizers are
already disabled for core.o, which is the only object file being built
in this sub-directory.

--- a/arch/x86/coco/sev/Makefile
+++ b/arch/x86/coco/sev/Makefile
@@ -13,3 +13,5 @@ KCOV_INSTRUMENT_core.o        := n
 # With some compiler versions the generated code results in boot hangs, caused
 # by several compilation units. To be safe, disable all instrumentation.
 KCSAN_SANITIZE         := n
+
+UBSAN_SANITIZE         := n