From: Ard Biesheuvel <ardb@kernel.org>

When retpolines and IBT are both disabled, the compiler is free to use
jump tables to optimize switch instructions. However, these are emitted
by Clang as absolute references into .rodata, e.g.,

        jmp    *-0x7dfffe90(,%r9,8)
                        R_X86_64_32S    .rodata+0x170

Given that this code will execute before that address in .rodata has even
been mapped, it is guaranteed to crash a SEV-SNP guest in a way that is
difficult to diagnose.

So disable jump tables when building this code. It would be better if we
could attach this annotation to the __head macro but this appears to be
impossible.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/x86/coco/sev/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/coco/sev/Makefile b/arch/x86/coco/sev/Makefile
index 08de37559307..dcb06dc8b5ae 100644
--- a/arch/x86/coco/sev/Makefile
+++ b/arch/x86/coco/sev/Makefile
@@ -2,6 +2,10 @@
 
 obj-y += core.o
 
+# jump tables are emitted using absolute references in non-PIC code
+# so they cannot be used in the early SEV startup code
+CFLAGS_core.o += -fno-jump-tables
+
 ifdef CONFIG_FUNCTION_TRACER
 CFLAGS_REMOVE_core.o = -pg
 endif
-- 
2.48.1.262.g85cc9f2d1e-goog

On Mon, 27 Jan 2025 at 03:43, Ard Biesheuvel <ardb+git@google.com> wrote:
>
> +# jump tables are emitted using absolute references in non-PIC code
> +# so they cannot be used in the early SEV startup code
> +CFLAGS_core.o += -fno-jump-tables

I can confirm that this looks like it fixes the problem for my
particular config.

Is the SEV code the only thing that needs this? And on the other hand,
isn't it just a _part_ of core.c that needs this? Maybe the "_head"
parts should be in a separate file?

I'm looking at (for example) arch/x86/mm/mem_encrypt_identity.c and
arch/x86/kernel/head64.c, and it looks like it really should have that
-fno-jump-tables thing too.

It just randomly may not have any switch tables or other things that
makes the compiler generate that code pattern.

(I did this just by a grep, maybe there's something else protecting
those files that I just didn't see)

           Linus

On Mon, 27 Jan 2025 at 18:10, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Mon, 27 Jan 2025 at 03:43, Ard Biesheuvel <ardb+git@google.com> wrote:
> >
> > +# jump tables are emitted using absolute references in non-PIC code
> > +# so they cannot be used in the early SEV startup code
> > +CFLAGS_core.o += -fno-jump-tables
>
> I can confirm that this looks like it fixes the problem for my
> particular config.
>

Great.

> Is the SEV code the only thing that needs this? And on the other hand,
> isn't it just a _part_ of core.c that needs this? Maybe the "_head"
> parts should be in a separate file?
>
> I'm looking at (for example) arch/x86/mm/mem_encrypt_identity.c and
> arch/x86/kernel/head64.c, and it looks like it really should have that
> -fno-jump-tables thing too.
>
> It just randomly may not have any switch tables or other things that
> makes the compiler generate that code pattern.
>

Yes. This is essentially whack-a-mole. And note that placing functions
in .head.text is a manual process, and so there may be other code that
is reachable via early code paths that remain unchecked.

The right way to go about this would be to duplicate what I did for
arm64 in arch/arm64/kernel/pi/, i.e., to create a special set of
source files that can be called into, but cannot call out to other
code unless it is part of the same pi/ corpus. These files are built
with an entirely different set of compiler options, -fPIC being one of
the notable ones. The resulting objects are checked for absolute
relocations in a similar fashion, and are therefore guaranteed to be
truly position independent (hence the 'pi') without any need for
runtime fixups. (Note that these objects are still linked into vmlinux
as usual, but due to the symbol name prefixes added using objcopy,
they are part of an isolated namespace)

The use case is also quite similar to the x86 one, as a matter of
fact: the initial mapping of the arm64 kernel is also 1:1, and some
non-trivial work is needed before the kernel's virtual mapping can be
created, and doing all of that in C was becoming intractable.

If this seems like a sensible approach for x86 as well, I can do some
exploration into the feasibility (keeping in mind that a lot of this
code is already being shared with the decompressor and the EFI stub
too, so it might take some effort to untangle)

On Mon, 27 Jan 2025 at 23:15, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Mon, 27 Jan 2025 at 18:10, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > On Mon, 27 Jan 2025 at 03:43, Ard Biesheuvel <ardb+git@google.com> wrote:
> > >
> > > +# jump tables are emitted using absolute references in non-PIC code
> > > +# so they cannot be used in the early SEV startup code
> > > +CFLAGS_core.o += -fno-jump-tables
> >
> > I can confirm that this looks like it fixes the problem for my
> > particular config.
> >
>
> Great.
>
> > Is the SEV code the only thing that needs this? And on the other hand,
> > isn't it just a _part_ of core.c that needs this? Maybe the "_head"
> > parts should be in a separate file?
> >
> > I'm looking at (for example) arch/x86/mm/mem_encrypt_identity.c and
> > arch/x86/kernel/head64.c, and it looks like it really should have that
> > -fno-jump-tables thing too.
> >
> > It just randomly may not have any switch tables or other things that
> > makes the compiler generate that code pattern.
> >
>
...
> The use case is also quite similar to the x86 one, as a matter of
> fact: the initial mapping of the arm64 kernel is also 1:1, and some
> non-trivial work is needed before the kernel's virtual mapping can be
> created, and doing all of that in C was becoming intractable.
>

Doing it in *assembler* was becoming intractable, in case that wasn't clear.

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID:     1105ab42a84bc11c62597005f78ccad2434fbd66
Gitweb:        https://git.kernel.org/tip/1105ab42a84bc11c62597005f78ccad2434fbd66
Author:        Ard Biesheuvel <ardb@kernel.org>
AuthorDate:    Mon, 27 Jan 2025 12:43:37 +01:00
Committer:     Ingo Molnar <mingo@kernel.org>
CommitterDate: Tue, 28 Jan 2025 23:10:29 +01:00

x86/sev: Disable jump tables in SEV startup code

When retpolines and IBT are both disabled, the compiler is free to use
jump tables to optimize switch instructions. However, these are emitted
by Clang as absolute references into .rodata:

        jmp    *-0x7dfffe90(,%r9,8)
                        R_X86_64_32S    .rodata+0x170

Given that this code will execute before that address in .rodata has even
been mapped, it is guaranteed to crash a SEV-SNP guest in a way that is
difficult to diagnose.

So disable jump tables when building this code. It would be better if we
could attach this annotation to the __head macro but this appears to be
impossible.

Reported-by: Linus Torvalds <torvalds@linux-foundation.org>
Tested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Link: https://lore.kernel.org/r/20250127114334.1045857-6-ardb+git@google.com
---
 arch/x86/coco/sev/Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/x86/coco/sev/Makefile b/arch/x86/coco/sev/Makefile
index 08de375..dcb06dc 100644
--- a/arch/x86/coco/sev/Makefile
+++ b/arch/x86/coco/sev/Makefile
@@ -2,6 +2,10 @@
 
 obj-y += core.o
 
+# jump tables are emitted using absolute references in non-PIC code
+# so they cannot be used in the early SEV startup code
+CFLAGS_core.o += -fno-jump-tables
+
 ifdef CONFIG_FUNCTION_TRACER
 CFLAGS_REMOVE_core.o = -pg
 endif