Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <cover.1664298147.git.demi@invisiblethingslab.com>
 <f3b624e99adfdbbfc1976a60a73a6b5950e1840d.1664298147.git.demi@invisiblethingslab.com>
 <282a225d-8782-0321-6f0e-19dd4510dc42@suse.com> <CAMj1kXFQNqsW5RfHGac-eGbosJHBybu6+-Fap_bi_kVxWNpGeg@mail.gmail.com>
 <YzcjeiOW8+i2Zxsd@itl-email> <CAMj1kXHBBbCNV3CLesqZi7ttmmi8y4tZ1KO5vievy_CJrU2o3Q@mail.gmail.com>
 <YzeaKjmls1YI/3ox@itl-email> <01d22092-8292-8ed7-ece7-9ca32d15bbce@suse.com>
 <YzxxXuovwQt3NskE@itl-email> <a0dc1158-01b1-4272-b86e-52f4996f0747@suse.com> <Yz3I2qwl243h9ZfZ@itl-email>
In-Reply-To: <Yz3I2qwl243h9ZfZ@itl-email>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Wed, 5 Oct 2022 23:28:29 +0200
Message-ID: <CAMj1kXHFi71SKQAQHEjZTLyp-YooRTYZ2-nqydRZA5hys7tkKw@mail.gmail.com>
Subject: Re: [PATCH v4 1/2] Avoid using EFI tables Xen may have clobbered
To:     Demi Marie Obenour <demi@invisiblethingslab.com>
Cc:     Jan Beulich <jbeulich@suse.com>, xen-devel@lists.xenproject.org,
        linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org,
        Juergen Gross <jgross@suse.com>,
        Stefano Stabellini <sstabellini@kernel.org>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>,
        Kees Cook <keescook@chromium.org>,
        Anton Vorontsov <anton@enomsg.org>,
        Colin Cross <ccross@android.com>,
        Tony Luck <tony.luck@intel.com>,
        =?UTF-8?Q?Marek_Marczykowski=2DG=C3=B3recki?= 
        <marmarek@invisiblethingslab.com>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, 5 Oct 2022 at 20:11, Demi Marie Obenour
<demi@invisiblethingslab.com> wrote:
>
> On Wed, Oct 05, 2022 at 08:15:07AM +0200, Jan Beulich wrote:
> > On 04.10.2022 17:46, Demi Marie Obenour wrote:
> > > Linux has a function called efi_mem_reserve() that is used to reserve
> > > EfiBootServicesData memory that contains e.g. EFI configuration tables.
> > > This function does not work under Xen because Xen could have already
> > > clobbered the memory.  efi_mem_reserve() not working is the whole reason
> > > for this thread, as it prevents EFI tables that are in
> > > EfiBootServicesData from being used under Xen.
> > >
> > > A much nicer approach would be for Xen to reserve boot services memory
> > > unconditionally, but provide a hypercall that dom0 could used to free
> > > the parts of EfiBootServicesData memory that are no longer needed.  This
> > > would allow efi_mem_reserve() to work normally.
> >
> > efi_mem_reserve() actually working would be a layering violation;
> > controlling the EFI memory map is entirely Xen's job.
>
> Doing this properly would require Xen to understand all of the EFI
> tables that could validly be in EfiBootServices* and which could be of
> interest to dom0.  It might (at least on some very buggy firmware)
> require a partial ACPI and/or SMBIOS implementation too, if the firmware
> decided to put an ACPI or SMBIOS table in EfiBootServices*.
>
> > As to the hypercall you suggest - I wouldn't mind its addition, but only
> > for the case when -mapbs is used. As I've indicated before, I'm of the
> > opinion that default behavior should be matching the intentions of the
> > spec, and the intention of EfiBootServices* is for the space to be
> > reclaimed. Plus I'm sure you realize there's a caveat with Dom0 using
> > that hypercall: It might use it for regions where data lives which it
> > wouldn't care about itself, but which an eventual kexec-ed (or alike)
> > entity would later want to consume. Code/data potentially usable by
> > _anyone_ between two resets of the system cannot legitimately be freed
> > (and hence imo is wrong to live in EfiBootServices* regions).
>
> I agree, but currently some such data *is* in EfiBootServices* regions,
> sadly.  When -mapbs is *not* used, I recommend uninstalling all of the
> configuration tables that point to EfiBootServicesData memory before
> freeing that memory.
>

That seems like a reasonable approach to me. Tables like MEMATTR or
RT_PROP are mostly relevant for bare metal where the host kernel maps
the runtime services, and in general, passing on these tables without
knowing what they do is kind of fishy anyway. You might even argue
that only known table types should be forwarded in the first place,
regardless of the memory type.