Received-SPF: pass (google.com: domain of linux-kernel+bounces-32072-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
MIME-Version: 1.0
References: <ZZ7YuEexYSaZYmLK@tassilo> <20240111223650.3502633-1-kevinloughlin@google.com>
 <20240115204634.GHZaWZqsVyU_fvn_RW@fat_crate.local> <CAMj1kXH=k26nNyB+LQJ7WUJgbD2f3PREyjCzSngMCGc+72XJ6w@mail.gmail.com>
 <20240117130557.GDZafQtfRyeVFbBUXA@fat_crate.local> <CAMj1kXEML99u7a8mZMhiWXXozsJYhVPaaTzYkegiSJcLw2pNJg@mail.gmail.com>
In-Reply-To: <CAMj1kXEML99u7a8mZMhiWXXozsJYhVPaaTzYkegiSJcLw2pNJg@mail.gmail.com>
From: Ard Biesheuvel <ardb@kernel.org>
Date: Sun, 21 Jan 2024 15:12:56 +0100
Message-ID: <CAMj1kXGO68zBNXtdxDXbcQ0U7NRFe4m9bF4L_qFmYZ6mkOvmww@mail.gmail.com>
Subject: Re: [RFC PATCH v2] x86/sev: enforce RIP-relative accesses in early
 SEV/SME code
To: Borislav Petkov <bp@alien8.de>
Cc: Kevin Loughlin <kevinloughlin@google.com>, Thomas Gleixner <tglx@linutronix.de>, 
	Ingo Molnar <mingo@redhat.com>, Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org, 
	"H. Peter Anvin" <hpa@zytor.com>, Andy Lutomirski <luto@kernel.org>, Peter Zijlstra <peterz@infradead.org>, 
	Nathan Chancellor <nathan@kernel.org>, Nick Desaulniers <ndesaulniers@google.com>, 
	Bill Wendling <morbo@google.com>, Justin Stitt <justinstitt@google.com>, 
	Tom Lendacky <thomas.lendacky@amd.com>, Michael Kelley <mikelley@microsoft.com>, 
	Pankaj Gupta <pankaj.gupta@amd.com>, Stephen Rothwell <sfr@canb.auug.org.au>, 
	Arnd Bergmann <arnd@arndb.de>, Steve Rutherford <srutherford@google.com>, 
	Alexander Shishkin <alexander.shishkin@linux.intel.com>, 
	Hou Wenlong <houwenlong.hwl@antgroup.com>, Vegard Nossum <vegard.nossum@oracle.com>, 
	Josh Poimboeuf <jpoimboe@kernel.org>, Yuntao Wang <ytcoode@gmail.com>, 
	Wang Jinchao <wangjinchao@xfusion.com>, David Woodhouse <dwmw@amazon.co.uk>, 
	Brian Gerst <brgerst@gmail.com>, Hugh Dickins <hughd@google.com>, Joerg Roedel <jroedel@suse.de>, 
	Randy Dunlap <rdunlap@infradead.org>, Bjorn Helgaas <bhelgaas@google.com>, 
	Dionna Glaze <dionnaglaze@google.com>, Brijesh Singh <brijesh.singh@amd.com>, 
	Michael Roth <michael.roth@amd.com>, 
	"Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>, linux-kernel@vger.kernel.org, 
	llvm@lists.linux.dev, linux-coco@lists.linux.dev, 
	Ashish Kalra <ashish.kalra@amd.com>, Andi Kleen <ak@linux.intel.com>, 
	Adam Dunlap <acdunlap@google.com>, Peter Gonda <pgonda@google.com>, Jacob Xu <jacobhxu@google.com>, 
	Sidharth Telang <sidtelang@google.com>
Content-Type: text/plain; charset="UTF-8"

On Wed, 17 Jan 2024 at 14:38, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 17 Jan 2024 at 14:06, Borislav Petkov <bp@alien8.de> wrote:
> >
> > On Wed, Jan 17, 2024 at 11:59:14AM +0100, Ard Biesheuvel wrote:
> ...
> > > On arm64, we use a separate pseudo-namespace for code that can run
> > > safely at any offset, using the __pi_ prefix (for Position
> > > Independent). Using symbol prefixing at the linker level, we ensure
> > > that __pi_ code can only call other __pi_ code, or code that has been
> > > made available to it via an explicit __pi_ prefixed alias. (Happy to
> > > elaborate more but we should find a smaller audience - your cc list is
> > > a tad long). Perhaps this is something we should explore on x86 as
> > > well (note that the EFI stub does something similar for architectures
> > > that link the EFI stub into the core kernel rather than into the
> > > decompressor)
> >
> > Grepping through the tree, is __pi_memcpy one example for that?
> >
>
> That is an example of a function that is known to be callable from any
> context, and so it is emitted with an alias that makes it accessible
> to other code that is position independent.
>
> There is some linker foo in arch/arm64/kernel/pi/Makefile that builds
> a couple of objects in PIC mode. The source symbols have ordinary
> names (even the external imports), but will be renamed by the linker
> to have a __pi_ prefix. The result is that those objects can only call
> into each other, or out to ordinary code that has been marked as __pi_
> as well.
>
> The entry into this code is
>
> arch/arm64/kernel/head.S:885:   bl      __pi_kaslr_early_init
>
> which is called before relocations have been applied, as that requires
> knowing how the kernel base address is randomized.

I've done a quick experiment with moving the early code that runs from
the 1:1 mapping into .pi.text, and getting modpost to warn about calls
from this section into other text sections.

My preliminary conclusion confirms that the SEV code is quite
problematic in this regard (which is the reason for this patch, so we
already knew that, of course).
Below are a couple of examples of how these calls fan out.

TL;DR I think we will need a way to build certain objects with -fPIC
(as we do in other places and on other architectures), but we should
add instrumentation to ensure that these issues can be detected at
build time. The SEV boot code is especially tricky here as very few
people can even test it, so applying this patch and hoping that the
compiler will never generate reachable code paths that only work
correctly when executed via the ordinary kernel virtual mapping is not
sufficient.


1)
WARNING: modpost: vmlinux: section mismatch in reference:
startup_64_pi+0x33 (section: .pi.text) -> sme_enable (section:
init.text)

leads to

WARNING: modpost: vmlinux: section mismatch in reference:
sme_enable+0x21 (section: .pi.text) -> snp_init (section: .init.text)
WARNING: modpost: vmlinux: section mismatch in reference:
sme_enable+0x99 (section: .pi.text) -> cc_set_mask (section:
init.text)
WARNING: modpost: vmlinux: section mismatch in reference:
sme_enable+0xf8 (section: .pi.text) -> cmdline_find_option (section:
text)
WARNING: modpost: vmlinux: section mismatch in reference:
sme_enable+0x10c (section: .pi.text) -> strncmp (section: .text)
WARNING: modpost: vmlinux: section mismatch in reference:
sme_enable+0x129 (section: .pi.text) -> snp_abort (section:
init.text)

2)
WARNING: modpost: vmlinux: section mismatch in reference:
startup_64_setup_env+0x27 (section: .pi.text) -> vc_no_ghcb (section:
init.text)

leads to

WARNING: modpost: vmlinux: section mismatch in reference:
vc_no_ghcb+0x46 (section: .pi.text) -> do_vc_no_ghcb (section:
init.text)

leads to

WARNING: modpost: vmlinux: section mismatch in reference:
do_vc_no_ghcb+0x55 (section: .pi.text) -> snp_cpuid (section: .text)
WARNING: modpost: vmlinux: section mismatch in reference:
do_vc_no_ghcb+0x66 (section: .pi.text) -> __sev_cpuid_hv_msr (section:
text)
WARNING: modpost: vmlinux: section mismatch in reference:
do_vc_no_ghcb+0xc1 (section: .pi.text) -> __x86_return_thunk (section:
text..__x86.return_thunk)
WARNING: modpost: vmlinux: section mismatch in reference:
do_vc_no_ghcb+0xd4 (section: .pi.text) -> sev_es_terminate (section:
text)
WARNING: modpost: vmlinux: section mismatch in reference:
do_vc_no_ghcb+0xd9 (section: .pi.text) -> __stack_chk_fail (section:
noinstr.text)

3)
WARNING: modpost: vmlinux: section mismatch in reference:
__startup_64+0x39d (section: .pi.text) -> early_snp_set_memory_shared
(section: .init.text)

leads to

WARNING: modpost: vmlinux: section mismatch in reference:
early_snp_set_memory_shared+0x18 (section: .pi.text) ->
early_set_pages_state (section: .text)

leads to

WARNING: modpost: vmlinux: section mismatch in reference:
early_set_pages_state+0x151 (section: .pi.text) -> __warn_printk
(section: .text)