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 stable@vger.kernel.org and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 435d1a471598752446a72ad1201b3c980526d869 Mon Sep 17 00:00:00 2001
From: Peter Jones pjones@redhat.com 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: stable@vger.kernel.org Signed-off-by: Peter Jones pjones@redhat.com Link: https://lore.kernel.org/r/20200615202408.2242614-1-pjones@redhat.com Signed-off-by: Ard Biesheuvel ardb@kernel.org
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig index e6fc022bc87e..3939699e62fe 100644 --- a/drivers/firmware/efi/Kconfig +++ b/drivers/firmware/efi/Kconfig @@ -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. diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index edc5d36caf54..5114cae4ec97 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -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)
On Mon, 29 Jun 2020 at 11:32, gregkh@linuxfoundation.org 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 stable@vger.kernel.org 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.
------------------ original commit in Linus's tree ------------------
From 435d1a471598752446a72ad1201b3c980526d869 Mon Sep 17 00:00:00 2001 From: Peter Jones pjones@redhat.com 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: stable@vger.kernel.org Signed-off-by: Peter Jones pjones@redhat.com Link: https://lore.kernel.org/r/20200615202408.2242614-1-pjones@redhat.com Signed-off-by: Ard Biesheuvel ardb@kernel.org
diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig index e6fc022bc87e..3939699e62fe 100644 --- a/drivers/firmware/efi/Kconfig +++ b/drivers/firmware/efi/Kconfig @@ -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 && ACPIdefault ACPI_TABLE_UPGRADEhelpAllow loading of an ACPI SSDT overlay from an EFI variable specifiedby a kernel command line option.See Documentation/admin-guide/acpi/ssdt-overlays.rst for moreinformation.diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index edc5d36caf54..5114cae4ec97 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -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)
On Mon, Jun 29, 2020 at 05:18:08PM +0200, Ard Biesheuvel wrote:
On Mon, 29 Jun 2020 at 11:32, gregkh@linuxfoundation.org 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 stable@vger.kernel.org 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.
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?
thanks,
greg k-h
On Mon, 29 Jun 2020 at 17:48, Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Mon, Jun 29, 2020 at 05:18:08PM +0200, Ard Biesheuvel wrote:
On Mon, 29 Jun 2020 at 11:32, gregkh@linuxfoundation.org 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 stable@vger.kernel.org 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.
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.
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.
Thanks, Ard.
On Mon, Jun 29, 2020 at 08:30:21PM +0200, Ard Biesheuvel wrote:
On Mon, 29 Jun 2020 at 17:48, Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Mon, Jun 29, 2020 at 05:18:08PM +0200, Ard Biesheuvel wrote:
On Mon, 29 Jun 2020 at 11:32, gregkh@linuxfoundation.org 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 stable@vger.kernel.org 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.
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.
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.
Ok, got it applied back to 4.9 now, thanks.
greg k-h
On Tue, 7 Jul 2020 at 17:10, Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Mon, Jun 29, 2020 at 08:30:21PM +0200, Ard Biesheuvel wrote:
On Mon, 29 Jun 2020 at 17:48, Greg Kroah-Hartman gregkh@linuxfoundation.org wrote:
On Mon, Jun 29, 2020 at 05:18:08PM +0200, Ard Biesheuvel wrote:
On Mon, 29 Jun 2020 at 11:32, gregkh@linuxfoundation.org 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 stable@vger.kernel.org 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.
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.
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.
Ok, got it applied back to 4.9 now, thanks.
Thanks Greg.
linux-stable-mirror@lists.linaro.org
-
Ard Biesheuvel -
Greg Kroah-Hartman -
gregkh＠linuxfoundation.org