Message-Id: <20220621153623.3786960-1-ardb@kernel.org>
Date:   Tue, 21 Jun 2022 17:36:14 +0200
From:   Ard Biesheuvel <ardb@...nel.org>
To:     linux-efi@...r.kernel.org
Cc:     linux-kernel@...r.kernel.org, Ard Biesheuvel <ardb@...nel.org>,
        Matthew Garrett <mjg59@...f.ucam.org>,
        Peter Jones <pjones@...hat.com>,
        Tony Luck <tony.luck@...el.com>,
        Kees Cook <keescook@...omium.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>
Subject: [PATCH v2 0/9] efi: Restructure EFI varstore driver

This is marked as a v2 given that it is a followup to a RFC patch I sent
last week [0]. Since nobody commented that removing the old sysfs
efivars interface is a bad idea, I went ahead and performed the cleanup
that this enables.

Some of the prerequisites of this work have been posted separately and
have been queued up in efi/next already, mainly to move other users away
from the efivar API which they were using in the wrong way, or without a
good reason. [1]

The current state of things is that efi-pstore, efivarfs and the efivars
sysfs interface all share a common support layer which manages a linked
list containing efivar_entry items describing each EFI variable that
this support layer assumes to be present in the EFI variable store
managed by the firmware.

This shared layer also contains an efivars_operations pointer, which
carries function pointers that refer to the underlying EFI get/set
variable routines, but can be superseded by other implementations
(currently, this is only implemented for Google x86 systems that
implement the GSMI interface)

Each user of this shared layer has its own linked list, which means they
all have a different view of the underlying variable store, even though
they might operate on the same variables. For EFI pstore related
variables in particular, manipulating these behind the back of the other
drivers is likely to result in fun.

This shared layer as well as its 3 different users all use a single
semaphore to mediate access to the individual linked lists as well as
the ops pointer.

The shared layer carries a substantial amount of 'business logic'
related to which EFI variables are relevant to the firmware, to limit
whether and how they may be manipulated. This aspect of the code is
only relevant when such variables can be manipulated arbitrarily, e.g.
by user space, but EFI pstore, for example, has no need for this, as it
uses its own GUIDed namespace for EFI variables, and does not permit
other variables to be manipulated.

The two remaining users are efivars sysfs and efivarfs, both of which
provide a cached view of these 'important' variables. Given that the
former has been deprecated for a long time, and given the potential
concerns around using both concurrently, let's get rid of the sysfs
based one.

Then, we can restructure the efivars API so that this business logic
can be incorporated into the efivarfs driver, leaving only a minimal
wrapper around the get/set variable calls, allowing the GSMI replacement
to remain in use, as well as mediate access to the different services
using the existing semaphore. This is mainly useful to ensure that
set_variable() calls do no invalidate an enumeration of the EFI
variables that is in progress using get_next_variable() by another task.

[0] https://lore.kernel.org/linux-efi/20220616124740.580708-1-ardb@kernel.org/T/#t
[1] https://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git/log/

Cc: Matthew Garrett <mjg59@...f.ucam.org>
Cc: Peter Jones <pjones@...hat.com>
Cc: Tony Luck <tony.luck@...el.com>
Cc: Kees Cook <keescook@...omium.org>
Cc: Thomas Gleixner <tglx@...utronix.de>
Cc: Ingo Molnar <mingo@...hat.com>
Cc: Borislav Petkov <bp@...en8.de>
Cc: Dave Hansen <dave.hansen@...ux.intel.com>

Ard Biesheuvel (9):
  pstore: Don't expose ECC metadata via pstore file system
  efi: vars: Don't drop lock in the middle of efivar_init()
  efi: vars: Add thin wrapper around EFI get/set variable interface
  efi: pstore: Omit efivars caching EFI varstore access layer
  efi: vars: Use locking version to iterate over efivars linked lists
  efi: vars: Drop __efivar_entry_iter() helper which is no longer used
  efi: vars: Remove deprecated 'efivars' sysfs interface
  efi: vars: Switch to new wrapper layer
  efi: vars: Move efivar caching layer into efivarfs

 Documentation/x86/x86_64/uefi.rst        |    2 +-
 arch/arm/configs/milbeaut_m10v_defconfig |    1 -
 arch/ia64/configs/bigsur_defconfig       |    1 -
 arch/ia64/configs/generic_defconfig      |    1 -
 arch/ia64/configs/gensparse_defconfig    |    1 -
 arch/ia64/configs/tiger_defconfig        |    1 -
 arch/ia64/configs/zx1_defconfig          |    1 -
 arch/x86/configs/i386_defconfig          |    1 -
 arch/x86/configs/x86_64_defconfig        |    1 -
 drivers/firmware/efi/Kconfig             |   12 -
 drivers/firmware/efi/Makefile            |    1 -
 drivers/firmware/efi/efi-pstore.c        |  389 ++-----
 drivers/firmware/efi/efi.c               |    1 +
 drivers/firmware/efi/efivars.c           |  671 -----------
 drivers/firmware/efi/vars.c              | 1219 +++-----------------
 fs/efivarfs/Makefile                     |    2 +-
 fs/efivarfs/internal.h                   |   40 +
 fs/efivarfs/super.c                      |   15 +-
 fs/efivarfs/vars.c                       |  742 ++++++++++++
 fs/pstore/inode.c                        |    2 +-
 include/linux/efi.h                      |   80 +-
 21 files changed, 1058 insertions(+), 2126 deletions(-)
 delete mode 100644 drivers/firmware/efi/efivars.c
 create mode 100644 fs/efivarfs/vars.c

-- 
2.35.1

Powered by blists - more mailing lists