WTF: patch "[PATCH] efi: Make it possible to disable efivar_ssdt entirely" was seriously submitted to be applied to the 5.7-stable tree?

The patch below was submitted to be applied to the 5.7-stable tree.

I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst.

I could be totally wrong, and if so, please respond to <[email protected]> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again.

------------------ original commit in Linus's tree ------------------

From 435d1a471598752446a72ad1201b3c980526d869 Mon Sep 17 00:00:00 2001
From: Peter Jones <[email protected]>
Date: Mon, 15 Jun 2020 16:24:08 -0400
Subject: [PATCH] efi: Make it possible to disable efivar_ssdt entirely

In most cases, such as CONFIG_ACPI_CUSTOM_DSDT and CONFIG_ACPI_TABLE_UPGRADE, boot-time modifications to firmware tables are tied to specific Kconfig options. Currently this is not the case for modifying the ACPI SSDT via the efivar_ssdt kernel command line option and associated EFI variable.

This patch adds CONFIG_EFI_CUSTOM_SSDT_OVERLAYS, which defaults disabled, in order to allow enabling or disabling that feature during the build.

Cc <[email protected]>

Signed-off-by Peter Jones <[email protected]>

Link https://lore.kernel.org/r/[email protected] open here

Signed-off-by Ard Biesheuvel <[email protected]>

drivers/firmware/efi/Kconfig +11 -0

index e6fc022bc87e..3939699e62fe 100644
@@ -278,3 +278,14 @@ config EFI_EARLYCON
        depends on SERIAL_EARLYCON && !ARM && !IA64
        select FONT_SUPPORT
        select ARCH_USE_MEMREMAP_PROT
+
+config EFI_CUSTOM_SSDT_OVERLAYS
+       bool "Load custom ACPI SSDT overlay from an EFI variable"
+       depends on EFI_VARS && ACPI
+       default ACPI_TABLE_UPGRADE
+       help
+         Allow loading of an ACPI SSDT overlay from an EFI variable specified
+         by a kernel command line option.
+
+         See Documentation/admin-guide/acpi/ssdt-overlays.rst for more
+         information.

drivers/firmware/efi/efi.c +1 -1

index edc5d36caf54..5114cae4ec97 100644
@@ -189,7 +189,7 @@ static void generic_ops_unregister(void)
        efivars_unregister(&generic_efivars);
 }
 
-#if IS_ENABLED(CONFIG_ACPI)
+#ifdef CONFIG_EFI_CUSTOM_SSDT_OVERLAYS
 #define EFIVAR_SSDT_NAME_MAX   16
 static char efivar_ssdt[EFIVAR_SSDT_NAME_MAX] __initdata;
 static int __init efivar_ssdt_setup(char *str)

Quoted 11 lines · On Mon, 29 Jun 2020 at 11:32, <[email protected]> wrote:

The patch below was submitted to be applied to the 5.7-stable tree.

I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst.

I could be totally wrong, and if so, please respond to <[email protected]> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again.

Without this patch, there is no way to disable sideloading of SSDTs via EFI variables, which is a security hole. The fact that this is not governed by the existing ACPI_TABLE_UPGRADE Kconfig option was an oversight, and so distros currently have this functionality enabled inadvertently (although most of them have the lockdown check incorporated as well)

SSDTs can manipulate any memory (even kernel memory that has been mapped read-only) by using SystemMemory OpRegions in _INI AML methods, and setting an EFI variable once will make this persist across reboots.

Quoted 20 lines · Ard Biesheuvel

Quoted 11 lines · On Mon, 29 Jun 2020 at 11:32, <[email protected]> wrote:

The patch below was submitted to be applied to the 5.7-stable tree.

I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst.

I could be totally wrong, and if so, please respond to <[email protected]> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again.

Without this patch, there is no way to disable sideloading of SSDTs via EFI variables, which is a security hole. The fact that this is not governed by the existing ACPI_TABLE_UPGRADE Kconfig option was an oversight, and so distros currently have this functionality enabled inadvertently (although most of them have the lockdown check incorporated as well)

Quoted 4 lines

SSDTs can manipulate any memory (even kernel memory that has been mapped read-only) by using SystemMemory OpRegions in _INI AML methods, and setting an EFI variable once will make this persist across reboots.

All of this was not in the description of the patch at all, how were we supposed to know this?

And this really looks like a new feature now that you are supporting something that we previously could not do. To know that this is a "fix" is not obvious :(

I'll go queue it up, but how far back should it go?

Quoted 22 lines · Greg Kroah-Hartman

Quoted 20 lines · Ard Biesheuvel

Quoted 11 lines · On Mon, 29 Jun 2020 at 11:32, <[email protected]> wrote:

The patch below was submitted to be applied to the 5.7-stable tree.

I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst.

I could be totally wrong, and if so, please respond to <[email protected]> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again.

Without this patch, there is no way to disable sideloading of SSDTs via EFI variables, which is a security hole. The fact that this is not governed by the existing ACPI_TABLE_UPGRADE Kconfig option was an oversight, and so distros currently have this functionality enabled inadvertently (although most of them have the lockdown check incorporated as well)

Quoted 8 lines

Quoted 4 lines

SSDTs can manipulate any memory (even kernel memory that has been mapped read-only) by using SystemMemory OpRegions in _INI AML methods, and setting an EFI variable once will make this persist across reboots.

All of this was not in the description of the patch at all, how were we supposed to know this?

Good point. This patch was the result of same off-list discussion, so it was obvious to those involved but not for anyone else.

Quoted 6 lines

And this really looks like a new feature now that you are supporting something that we previously could not do. To know that this is a "fix" is not obvious :(

I'll go queue it up, but how far back should it go?

The feature was added in v4.8, so as close as we can get to that please.

Quoted 40 lines · Ard Biesheuvel

Quoted 22 lines · Greg Kroah-Hartman

Quoted 20 lines · Ard Biesheuvel

Quoted 11 lines · On Mon, 29 Jun 2020 at 11:32, <[email protected]> wrote:

The patch below was submitted to be applied to the 5.7-stable tree.

I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst.

I could be totally wrong, and if so, please respond to <[email protected]> and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again.

Without this patch, there is no way to disable sideloading of SSDTs via EFI variables, which is a security hole. The fact that this is not governed by the existing ACPI_TABLE_UPGRADE Kconfig option was an oversight, and so distros currently have this functionality enabled inadvertently (although most of them have the lockdown check incorporated as well)

Quoted 8 lines

Quoted 4 lines

SSDTs can manipulate any memory (even kernel memory that has been mapped read-only) by using SystemMemory OpRegions in _INI AML methods, and setting an EFI variable once will make this persist across reboots.

All of this was not in the description of the patch at all, how were we supposed to know this?

Good point. This patch was the result of same off-list discussion, so it was obvious to those involved but not for anyone else.

Quoted 4 lines

And this really looks like a new feature now that you are supporting something that we previously could not do. To know that this is a "fix" is not obvious :(

Quoted 4 lines

Quoted 2 lines

I'll go queue it up, but how far back should it go?

The feature was added in v4.8, so as close as we can get to that please.

Ok, got it applied back to 4.9 now, thanks.