Received-SPF: pass (google.com: domain of linux-kernel+bounces-94316-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Precedence: bulk
MIME-Version: 1.0
References: <CAMj1kXH1oMbONoHFMPaatfaqrHNE2ryfrG7kw-7J-eFsuXkK-Q@mail.gmail.com>
 <mhng-c53211c1-4708-459a-bdc5-6e013c2adaee@palmer-ri-x1c9>
 <CAMj1kXEFqMx8qUHQEbg=OqbG-H0Zpj-nWu=a6qhhvNEZPO7f4Q@mail.gmail.com> <CAMj1kXG4SXsBfNqWMRUJ+AVv=6trWUAow-f8Mk5oKCpO=WueFg@mail.gmail.com>
In-Reply-To: <CAMj1kXG4SXsBfNqWMRUJ+AVv=6trWUAow-f8Mk5oKCpO=WueFg@mail.gmail.com>
From: Ard Biesheuvel <ardb@kernel.org>
Date: Wed, 6 Mar 2024 18:11:57 +0100
Message-ID: <CAMj1kXHcisgPkkmkEM8AGDkmW64B-Sfp96ag457xV9uqxy3yVg@mail.gmail.com>
Subject: Re: [External] Re: [PATCH 3/3] efistub: fix missed the initialization
 of gp
To: Palmer Dabbelt <palmer@dabbelt.com>
Cc: cuiyunhui@bytedance.com, Paul Walmsley <paul.walmsley@sifive.com>, 
	aou@eecs.berkeley.edu, xuzhipeng.1973@bytedance.com, alexghiti@rivosinc.com, 
	samitolvanen@google.com, bp@alien8.de, xiao.w.wang@intel.com, 
	jan.kiszka@siemens.com, kirill.shutemov@linux.intel.com, nathan@kernel.org, 
	linux-riscv@lists.infradead.org, linux-kernel@vger.kernel.org, 
	linux-efi@vger.kernel.org, Conor Dooley <conor@kernel.org>, 
	Heinrich Schuchardt <heinrich.schuchardt@canonical.com>, Pedro Falcato <pedro.falcato@gmail.com>
Content-Type: text/plain; charset="UTF-8"

On Wed, 6 Mar 2024 at 17:15, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Wed, 6 Mar 2024 at 16:44, Ard Biesheuvel <ardb@kernel.org> wrote:
> >
> > On Wed, 6 Mar 2024 at 16:21, Palmer Dabbelt <palmer@dabbelt.com> wrote:
> > >
..
> > >
> > > The UEFI spec says "UEFI firmware must neither trust the
> > > values of tp and gp nor make an assumption of owning the write access to
> > > these register in any circumstances".  It's kind of vague what "UEFI
> > > firmware" means here, but I think it's reasonable to assume that the
> > > kernel (and thus the EFI stub) is not included there.
> > >
> > > So under that interpretation, the kernel (including the EFI stub) would
> > > be allowed to overwrite GP with whatever it wants.
> > >
..

After some more consideration, I concluded that using GP in code that
executes in the context of EFI is never safe.

Taking the typical Linux/EFI boot sequence as an example, where GRUB
is loaded by the system firmware, and the Linux EFI stub is loaded by
GRUB, it is easy to spot the problem: GRUB exposes the initial ramdisk
via the LoadFile2 protocol, which is essentially a callback interface.

If we assume that EFI apps can use GP as they like, we cannot safely
call this callback interface without restoring GP to its original
value, since we have no idea whether GRUB (or some other loader) is
relying on its value. And in addition to synchronous callbacks, there
may be EFI event callbacks registered by GRUB that are signaled by the
system firmware asynchronously, which means GRUB code could be called
behind our backs. Without any guidance in the UEFI spec on how GP
needs to be managed across such boundaries, we should assume that GP
needs to be restored to the old value in each of those cases, but this
is impossible for async event handlers.

We might conclude then that using GP is only safe for EFI apps if they
don't install protocols or register for EFI events, but that would
still imply that we should restore GP to its old value upon exit.

So without a strong argument why GP relaxations need to be supported
in the EFI stub, I don't think we should consider these changes. If
there are any issues in the current code that result in inadvertent GP
relaxations, we should address those instead.