public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ardb@kernel.org>
To: linux-efi@vger.kernel.org
Cc: linux-kernel@vger.kernel.org, Ard Biesheuvel <ardb@kernel.org>,
	Matthew Garrett <mjg59@srcf.ucam.org>,
	Peter Jones <pjones@redhat.com>, Tony Luck <tony.luck@intel.com>,
	Kees Cook <keescook@chromium.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	Dave Hansen <dave.hansen@linux.intel.com>
Subject: [PATCH v2 0/9] efi: Restructure EFI varstore driver
Date: Tue, 21 Jun 2022 17:36:14 +0200	[thread overview]
Message-ID: <20220621153623.3786960-1-ardb@kernel.org> (raw)

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@srcf.ucam.org>
Cc: Peter Jones <pjones@redhat.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.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


             reply	other threads:[~2022-06-21 15:36 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-06-21 15:36 Ard Biesheuvel [this message]
2022-06-21 15:36 ` [PATCH v2 1/9] pstore: Don't expose ECC metadata via pstore file system Ard Biesheuvel
2022-06-21 21:19   ` Kees Cook
2022-06-21 15:36 ` [PATCH v2 2/9] efi: vars: Don't drop lock in the middle of efivar_init() Ard Biesheuvel
2022-06-21 15:36 ` [PATCH v2 3/9] efi: vars: Add thin wrapper around EFI get/set variable interface Ard Biesheuvel
2022-06-21 15:36 ` [PATCH v2 4/9] efi: pstore: Omit efivars caching EFI varstore access layer Ard Biesheuvel
2022-06-21 21:00   ` Kees Cook
2022-06-21 21:12     ` Ard Biesheuvel
2022-06-21 21:21       ` Kees Cook
2022-06-21 21:33         ` Ard Biesheuvel
2022-06-21 22:00           ` Kees Cook
2022-06-21 15:36 ` [PATCH v2 5/9] efi: vars: Use locking version to iterate over efivars linked lists Ard Biesheuvel
2022-06-21 15:36 ` [PATCH v2 6/9] efi: vars: Drop __efivar_entry_iter() helper which is no longer used Ard Biesheuvel
2022-06-21 15:36 ` [PATCH v2 7/9] efi: vars: Remove deprecated 'efivars' sysfs interface Ard Biesheuvel
2022-06-21 15:36 ` [PATCH v2 8/9] efi: vars: Switch to new wrapper layer Ard Biesheuvel
2022-06-21 15:36 ` [PATCH v2 9/9] efi: vars: Move efivar caching layer into efivarfs Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220621153623.3786960-1-ardb@kernel.org \
    --to=ardb@kernel.org \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=keescook@chromium.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=mjg59@srcf.ucam.org \
    --cc=pjones@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=tony.luck@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox