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

[Nathan Chancellor <nathan@kernel.org>]

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@alien8.de> 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.

[1]: https://github.com/llvm/llvm-project/commit/92c1bc61586c9d6c7bf0c36b1005fe00b4f48cc0
[2]: https://github.com/llvm/llvm-project/issues/56568
[3]: https://godbolt.org/z/cxhW5orxr

Cheers,
Nathan

diff --git a/arch/x86/include/asm/sev.h b/arch/x86/include/asm/sev.h
index 91f08af31078..7887bac1fbab 100644
--- a/arch/x86/include/asm/sev.h
+++ b/arch/x86/include/asm/sev.h
@@ -414,7 +414,7 @@ static inline int rmpadjust(unsigned long vaddr, bool rmp_psize, unsigned long a
 
 	return rc;
 }
-static inline int pvalidate(unsigned long vaddr, bool rmp_psize, bool validate)
+static inline __no_sanitize_undefined int pvalidate(unsigned long vaddr, bool rmp_psize, bool validate)
 {
 	bool no_rmpupdate;
 	int rc;