Re: [PATCH] ACPI: PRM: Check whether EFI runtime is available

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jan 11, 2023 at 10:17 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
>
> On Wed, 11 Jan 2023 at 21:23, Rafael J. Wysocki <rafael@xxxxxxxxxx> wrote:
> >
> > On Wed, Jan 11, 2023 at 2:27 PM Ard Biesheuvel <ardb@xxxxxxxxxx> wrote:
> > >
> > > The ACPI PRM address space handler calls efi_call_virt_pointer() to
> > > execute PRM firmware code, but doing so is only permitted when the EFI
> > > runtime environment is available. Otherwise, such calls are guaranteed
> > > to result in a crash, and must therefore be avoided.
> > >
> > > Cc: <stable@xxxxxxxxxxxxxxx>
> > > Cc: "Rafael J. Wysocki" <rafael@xxxxxxxxxx>
> > > Cc: Len Brown <lenb@xxxxxxxxxx>
> > > Cc: linux-acpi@xxxxxxxxxxxxxxx
> > > Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>
> > > ---
> > >  drivers/acpi/prmt.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/drivers/acpi/prmt.c b/drivers/acpi/prmt.c
> > > index 998101cf16e47145..74f924077866ae69 100644
> > > --- a/drivers/acpi/prmt.c
> > > +++ b/drivers/acpi/prmt.c
> > > @@ -236,6 +236,11 @@ static acpi_status acpi_platformrt_space_handler(u32 function,
> > >         efi_status_t status;
> > >         struct prm_context_buffer context;
> > >
> > > +       if (!efi_enabled(EFI_RUNTIME_SERVICES)) {
> > > +               pr_err("PRM: EFI runtime services unavailable\n");
> > > +               return AE_NOT_IMPLEMENTED;
> > > +       }
> > > +
> >
> > Does the check need to be made in the address space handler and if so, then why?
> >
>
> Yes. efi_enabled(EFI_RUNTIME_SERVICES) will transition from true to
> false if an exception occurs while executing the firmware code.

OK

> Unlike the EFI variable runtime services, which are quite uniform,
> this PRM code will be vendor specific, and so the likelihood that it
> is buggy and only tested with Windows is much higher, and so I would
> like us to be more cautious here.

OK

> > It looks like it would be better to prevent it from being installed if
> > EFI runtime services are not enabled in the first place, in
> > init_prmt().
> >
>
> We could add another check there as well, yes. And perhaps the one in
> the handler should we pr_warn_once() to prevent it from firing
> repeatedly.

Sounds good to me, so are you going to send an update of the patch?
Or how would you like to proceed otherwise?

> > >         /*
> > >          * The returned acpi_status will always be AE_OK. Error values will be
> > >          * saved in the first byte of the PRM message buffer to be used by ASL.
> > > --



[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux