From: "Rafael J. Wysocki" <rjw@rjwysocki.net> To: Matthew Garrett <matthewgarrett@google.com> Cc: jmorris@namei.org, linux-security-module@vger.kernel.org, linux-kernel@vger.kernel.org, linux-api@vger.kernel.org, Josh Boyer <jwboyer@redhat.com>, David Howells <dhowells@redhat.com>, Matthew Garrett <mjg59@google.com>, Kees Cook <keescook@chromium.org>, Dave Young <dyoung@redhat.com>, linux-acpi@vger.kernel.org Subject: Re: [PATCH V40 15/29] acpi: Ignore acpi_rsdp kernel param when the kernel has been locked down Date: Wed, 21 Aug 2019 00:08:18 +0200 Message-ID: <8258877.02sui5J5Gm@kreacher> (raw) In-Reply-To: <20190820001805.241928-16-matthewgarrett@google.com> On Tuesday, August 20, 2019 2:17:51 AM CEST Matthew Garrett wrote: > From: Josh Boyer <jwboyer@redhat.com> > > This option allows userspace to pass the RSDP address to the kernel, which > makes it possible for a user to modify the workings of hardware. Reject > the option when the kernel is locked down. This requires some reworking > of the existing RSDP command line logic, since the early boot code also > makes use of a command-line passed RSDP when locating the SRAT table > before the lockdown code has been initialised. This is achieved by > separating the command line RSDP path in the early boot code from the > generic RSDP path, and then copying the command line RSDP into boot > params in the kernel proper if lockdown is not enabled. If lockdown is > enabled and an RSDP is provided on the command line, this will only be > used when parsing SRAT (which shouldn't permit kernel code execution) > and will be ignored in the rest of the kernel. > > (Modified by Matthew Garrett in order to handle the early boot RSDP > environment) > > Signed-off-by: Josh Boyer <jwboyer@redhat.com> > Signed-off-by: David Howells <dhowells@redhat.com> > Signed-off-by: Matthew Garrett <mjg59@google.com> > Reviewed-by: Kees Cook <keescook@chromium.org> > cc: Dave Young <dyoung@redhat.com> > cc: linux-acpi@vger.kernel.org > Signed-off-by: James Morris <jmorris@namei.org> Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > arch/x86/boot/compressed/acpi.c | 19 +++++++++++++------ > arch/x86/include/asm/acpi.h | 9 +++++++++ > arch/x86/include/asm/x86_init.h | 2 ++ > arch/x86/kernel/acpi/boot.c | 5 +++++ > arch/x86/kernel/x86_init.c | 1 + > drivers/acpi/osl.c | 14 +++++++++++++- > include/linux/acpi.h | 6 ++++++ > 7 files changed, 49 insertions(+), 7 deletions(-) > > diff --git a/arch/x86/boot/compressed/acpi.c b/arch/x86/boot/compressed/acpi.c > index ad84239e595e..e726e9b44bb1 100644 > --- a/arch/x86/boot/compressed/acpi.c > +++ b/arch/x86/boot/compressed/acpi.c > @@ -26,7 +26,7 @@ struct mem_vector immovable_mem[MAX_NUMNODES*2]; > */ > #define MAX_ADDR_LEN 19 > > -static acpi_physical_address get_acpi_rsdp(void) > +static acpi_physical_address get_cmdline_acpi_rsdp(void) > { > acpi_physical_address addr = 0; > > @@ -215,10 +215,7 @@ acpi_physical_address get_rsdp_addr(void) > { > acpi_physical_address pa; > > - pa = get_acpi_rsdp(); > - > - if (!pa) > - pa = boot_params->acpi_rsdp_addr; > + pa = boot_params->acpi_rsdp_addr; > > if (!pa) > pa = efi_get_rsdp_addr(); > @@ -240,7 +237,17 @@ static unsigned long get_acpi_srat_table(void) > char arg[10]; > u8 *entry; > > - rsdp = (struct acpi_table_rsdp *)(long)boot_params->acpi_rsdp_addr; > + /* > + * Check whether we were given an RSDP on the command line. We don't > + * stash this in boot params because the kernel itself may have > + * different ideas about whether to trust a command-line parameter. > + */ > + rsdp = (struct acpi_table_rsdp *)get_cmdline_acpi_rsdp(); > + > + if (!rsdp) > + rsdp = (struct acpi_table_rsdp *)(long) > + boot_params->acpi_rsdp_addr; > + > if (!rsdp) > return 0; > > diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h > index aac686e1e005..bc9693c9107e 100644 > --- a/arch/x86/include/asm/acpi.h > +++ b/arch/x86/include/asm/acpi.h > @@ -117,6 +117,12 @@ static inline bool acpi_has_cpu_in_madt(void) > return !!acpi_lapic; > } > > +#define ACPI_HAVE_ARCH_SET_ROOT_POINTER > +static inline void acpi_arch_set_root_pointer(u64 addr) > +{ > + x86_init.acpi.set_root_pointer(addr); > +} > + > #define ACPI_HAVE_ARCH_GET_ROOT_POINTER > static inline u64 acpi_arch_get_root_pointer(void) > { > @@ -125,6 +131,7 @@ static inline u64 acpi_arch_get_root_pointer(void) > > void acpi_generic_reduced_hw_init(void); > > +void x86_default_set_root_pointer(u64 addr); > u64 x86_default_get_root_pointer(void); > > #else /* !CONFIG_ACPI */ > @@ -138,6 +145,8 @@ static inline void disable_acpi(void) { } > > static inline void acpi_generic_reduced_hw_init(void) { } > > +static inline void x86_default_set_root_pointer(u64 addr) { } > + > static inline u64 x86_default_get_root_pointer(void) > { > return 0; > diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h > index b85a7c54c6a1..d584128435cb 100644 > --- a/arch/x86/include/asm/x86_init.h > +++ b/arch/x86/include/asm/x86_init.h > @@ -134,10 +134,12 @@ struct x86_hyper_init { > > /** > * struct x86_init_acpi - x86 ACPI init functions > + * @set_root_poitner: set RSDP address > * @get_root_pointer: get RSDP address > * @reduced_hw_early_init: hardware reduced platform early init > */ > struct x86_init_acpi { > + void (*set_root_pointer)(u64 addr); > u64 (*get_root_pointer)(void); > void (*reduced_hw_early_init)(void); > }; > diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c > index 17b33ef604f3..04205ce127a1 100644 > --- a/arch/x86/kernel/acpi/boot.c > +++ b/arch/x86/kernel/acpi/boot.c > @@ -1760,6 +1760,11 @@ void __init arch_reserve_mem_area(acpi_physical_address addr, size_t size) > e820__update_table_print(); > } > > +void x86_default_set_root_pointer(u64 addr) > +{ > + boot_params.acpi_rsdp_addr = addr; > +} > + > u64 x86_default_get_root_pointer(void) > { > return boot_params.acpi_rsdp_addr; > diff --git a/arch/x86/kernel/x86_init.c b/arch/x86/kernel/x86_init.c > index 50a2b492fdd6..d0b8f5585a73 100644 > --- a/arch/x86/kernel/x86_init.c > +++ b/arch/x86/kernel/x86_init.c > @@ -95,6 +95,7 @@ struct x86_init_ops x86_init __initdata = { > }, > > .acpi = { > + .set_root_pointer = x86_default_set_root_pointer, > .get_root_pointer = x86_default_get_root_pointer, > .reduced_hw_early_init = acpi_generic_reduced_hw_init, > }, > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c > index cc7507091dec..b7c3aeb175dd 100644 > --- a/drivers/acpi/osl.c > +++ b/drivers/acpi/osl.c > @@ -26,6 +26,7 @@ > #include <linux/list.h> > #include <linux/jiffies.h> > #include <linux/semaphore.h> > +#include <linux/security.h> > > #include <asm/io.h> > #include <linux/uaccess.h> > @@ -180,8 +181,19 @@ acpi_physical_address __init acpi_os_get_root_pointer(void) > acpi_physical_address pa; > > #ifdef CONFIG_KEXEC > - if (acpi_rsdp) > + /* > + * We may have been provided with an RSDP on the command line, > + * but if a malicious user has done so they may be pointing us > + * at modified ACPI tables that could alter kernel behaviour - > + * so, we check the lockdown status before making use of > + * it. If we trust it then also stash it in an architecture > + * specific location (if appropriate) so it can be carried > + * over further kexec()s. > + */ > + if (acpi_rsdp && !security_locked_down(LOCKDOWN_ACPI_TABLES)) { > + acpi_arch_set_root_pointer(acpi_rsdp); > return acpi_rsdp; > + } > #endif > pa = acpi_arch_get_root_pointer(); > if (pa) > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index d315d86844e4..268a4d91f54c 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -632,6 +632,12 @@ bool acpi_gtdt_c3stop(int type); > int acpi_arch_timer_mem_init(struct arch_timer_mem *timer_mem, int *timer_count); > #endif > > +#ifndef ACPI_HAVE_ARCH_SET_ROOT_POINTER > +static inline void acpi_arch_set_root_pointer(u64 addr) > +{ > +} > +#endif > + > #ifndef ACPI_HAVE_ARCH_GET_ROOT_POINTER > static inline u64 acpi_arch_get_root_pointer(void) > { >
next prev parent reply index Thread overview: 46+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-08-20 0:17 [PATCH V40 00/29] Add kernel lockdown functionality Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 01/29] security: Support early LSMs Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 02/29] security: Add a "locked down" LSM hook Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 03/29] security: Add a static lockdown policy LSM Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 04/29] lockdown: Enforce module signatures if the kernel is locked down Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 05/29] lockdown: Restrict /dev/{mem,kmem,port} when " Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 06/29] kexec_load: Disable at runtime if " Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 07/29] lockdown: Copy secure_boot flag in boot params across kexec reboot Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 08/29] kexec_file: split KEXEC_VERIFY_SIG into KEXEC_SIG and KEXEC_SIG_FORCE Matthew Garrett 2019-08-30 14:26 ` Philipp Rudo 2019-08-20 0:17 ` [PATCH V40 09/29] kexec_file: Restrict at runtime if the kernel is locked down Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 10/29] hibernate: Disable when " Matthew Garrett 2019-08-20 21:43 ` Rafael J. Wysocki 2019-08-25 9:51 ` Pavel Machek 2019-08-20 0:17 ` [PATCH V40 11/29] PCI: Lock down BAR access " Matthew Garrett 2019-08-20 19:45 ` Bjorn Helgaas 2019-08-20 21:04 ` Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 12/29] x86: Lock down IO port " Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 13/29] x86/msr: Restrict MSR " Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 14/29] ACPI: Limit access to custom_method " Matthew Garrett 2019-08-20 22:07 ` Rafael J. Wysocki 2019-08-20 0:17 ` [PATCH V40 15/29] acpi: Ignore acpi_rsdp kernel param when the kernel has been " Matthew Garrett 2019-08-20 22:08 ` Rafael J. Wysocki [this message] 2019-08-20 0:17 ` [PATCH V40 16/29] acpi: Disable ACPI table override if the kernel is " Matthew Garrett 2019-08-20 22:08 ` Rafael J. Wysocki 2019-08-20 0:17 ` [PATCH V40 17/29] lockdown: Prohibit PCMCIA CIS storage when " Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 18/29] lockdown: Lock down TIOCSSERIAL Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 19/29] lockdown: Lock down module params that specify hardware parameters (eg. ioport) Matthew Garrett 2019-08-20 16:39 ` Jessica Yu 2019-08-20 0:17 ` [PATCH V40 20/29] x86/mmiotrace: Lock down the testmmiotrace module Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 21/29] lockdown: Lock down /proc/kcore Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 22/29] lockdown: Lock down tracing and perf kprobes when in confidentiality mode Matthew Garrett 2019-08-20 0:17 ` [PATCH V40 23/29] bpf: Restrict bpf when kernel lockdown is " Matthew Garrett 2019-08-20 0:18 ` [PATCH V40 24/29] lockdown: Lock down perf when " Matthew Garrett 2019-08-20 0:18 ` [PATCH V40 25/29] kexec: Allow kexec_file() with appropriate IMA policy when locked down Matthew Garrett 2019-08-20 0:18 ` [PATCH V40 26/29] debugfs: Restrict debugfs when the kernel is " Matthew Garrett 2019-08-20 0:18 ` [PATCH V40 27/29] tracefs: Restrict tracefs " Matthew Garrett 2019-08-20 0:18 ` [PATCH V40 28/29] efi: Restrict efivar_ssdt_load " Matthew Garrett 2019-08-20 0:18 ` [PATCH V40 29/29] lockdown: Print current->comm in restriction messages Matthew Garrett 2019-08-20 6:45 ` [PATCH V40 00/29] Add kernel lockdown functionality James Morris 2019-08-30 16:28 ` [PATCH V40 03/29] security: Add a static lockdown policy LSM David Howells 2019-09-04 16:51 ` Matthew Garrett 2019-09-10 10:06 ` Matthew Garrett 2019-08-30 16:31 ` [PATCH V40 04/29] lockdown: Enforce module signatures if the kernel is locked down David Howells 2019-09-04 16:57 ` Matthew Garrett 2019-08-30 16:32 ` [PATCH V40 23/29] bpf: Restrict bpf when kernel lockdown is in confidentiality mode David Howells
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=8258877.02sui5J5Gm@kreacher \ --to=rjw@rjwysocki.net \ --cc=dhowells@redhat.com \ --cc=dyoung@redhat.com \ --cc=jmorris@namei.org \ --cc=jwboyer@redhat.com \ --cc=keescook@chromium.org \ --cc=linux-acpi@vger.kernel.org \ --cc=linux-api@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-security-module@vger.kernel.org \ --cc=matthewgarrett@google.com \ --cc=mjg59@google.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
Linux-Security-Module Archive on lore.kernel.org Archives are clonable: git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git # If you have public-inbox 1.1+ installed, you may # initialize and index your mirror using the following commands: public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \ linux-security-module@vger.kernel.org public-inbox-index linux-security-module Example config snippet for mirrors Newsgroup available over NNTP: nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module AGPL code for this site: git clone https://public-inbox.org/public-inbox.git