From 1a3887924a7e6edd331be76da7bf4c1e8eab4b1e Mon Sep 17 00:00:00 2001 From: Ard Biesheuvel Date: Mon, 22 Aug 2022 19:20:33 +0200 Subject: efi: libstub: Disable struct randomization The EFI stub is a wrapper around the core kernel that makes it look like a EFI compatible PE/COFF application to the EFI firmware. EFI applications run on top of the EFI runtime, which is heavily based on so-called protocols, which are struct types consisting [mostly] of function pointer members that are instantiated and recorded in a protocol database. These structs look like the ideal randomization candidates to the randstruct plugin (as they only carry function pointers), but of course, these protocols are contracts between the firmware that exposes them, and the EFI applications (including our stubbed kernel) that invoke them. This means that struct randomization for EFI protocols is not a great idea, and given that the stub shares very little data with the core kernel that is represented as a randomizable struct, we're better off just disabling it completely here. Cc: # v4.14+ Reported-by: Daniel Marth Tested-by: Daniel Marth Signed-off-by: Ard Biesheuvel Acked-by: Kees Cook --- drivers/firmware/efi/libstub/Makefile | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'drivers') diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile index d0537573501e..2c67f71f2375 100644 --- a/drivers/firmware/efi/libstub/Makefile +++ b/drivers/firmware/efi/libstub/Makefile @@ -37,6 +37,13 @@ KBUILD_CFLAGS := $(cflags-y) -Os -DDISABLE_BRANCH_PROFILING \ $(call cc-option,-fno-addrsig) \ -D__DISABLE_EXPORTS +# +# struct randomization only makes sense for Linux internal types, which the EFI +# stub code never touches, so let's turn off struct randomization for the stub +# altogether +# +KBUILD_CFLAGS := $(filter-out $(RANDSTRUCT_CFLAGS), $(KBUILD_CFLAGS)) + # remove SCS flags from all objects in this directory KBUILD_CFLAGS := $(filter-out $(CC_FLAGS_SCS), $(KBUILD_CFLAGS)) # disable LTO -- cgit v1.2.3 From 7a1ec84ffba9e90ac772ddb33ea9c3899ed8d2c9 Mon Sep 17 00:00:00 2001 From: chen zhang Date: Wed, 7 Sep 2022 10:32:55 +0800 Subject: efi/x86: libstub: remove unused variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The variable "has_system_memory" is unused in function ‘adjust_memory_range_protection’, remove it. Signed-off-by: chen zhang Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/libstub/x86-stub.c | 1 - 1 file changed, 1 deletion(-) (limited to 'drivers') diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c index 05ae8bcc9d67..43ca665af610 100644 --- a/drivers/firmware/efi/libstub/x86-stub.c +++ b/drivers/firmware/efi/libstub/x86-stub.c @@ -220,7 +220,6 @@ adjust_memory_range_protection(unsigned long start, unsigned long size) unsigned long end, next; unsigned long rounded_start, rounded_end; unsigned long unprotect_start, unprotect_size; - int has_system_memory = 0; if (efi_dxe_table == NULL) return; -- cgit v1.2.3 From 9cb636b5f6a8cc6d1b50809ec8f8d33ae0c84c95 Mon Sep 17 00:00:00 2001 From: Hyunwoo Kim Date: Wed, 7 Sep 2022 09:07:14 -0700 Subject: efi: capsule-loader: Fix use-after-free in efi_capsule_write A race condition may occur if the user calls close() on another thread during a write() operation on the device node of the efi capsule. This is a race condition that occurs between the efi_capsule_write() and efi_capsule_flush() functions of efi_capsule_fops, which ultimately results in UAF. So, the page freeing process is modified to be done in efi_capsule_release() instead of efi_capsule_flush(). Cc: # v4.9+ Signed-off-by: Hyunwoo Kim Link: https://lore.kernel.org/all/20220907102920.GA88602@ubuntu/ Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/capsule-loader.c | 31 +++++++------------------------ 1 file changed, 7 insertions(+), 24 deletions(-) (limited to 'drivers') diff --git a/drivers/firmware/efi/capsule-loader.c b/drivers/firmware/efi/capsule-loader.c index 4dde8edd53b6..3e8d4b51a814 100644 --- a/drivers/firmware/efi/capsule-loader.c +++ b/drivers/firmware/efi/capsule-loader.c @@ -242,29 +242,6 @@ failed: return ret; } -/** - * efi_capsule_flush - called by file close or file flush - * @file: file pointer - * @id: not used - * - * If a capsule is being partially uploaded then calling this function - * will be treated as upload termination and will free those completed - * buffer pages and -ECANCELED will be returned. - **/ -static int efi_capsule_flush(struct file *file, fl_owner_t id) -{ - int ret = 0; - struct capsule_info *cap_info = file->private_data; - - if (cap_info->index > 0) { - pr_err("capsule upload not complete\n"); - efi_free_all_buff_pages(cap_info); - ret = -ECANCELED; - } - - return ret; -} - /** * efi_capsule_release - called by file close * @inode: not used @@ -277,6 +254,13 @@ static int efi_capsule_release(struct inode *inode, struct file *file) { struct capsule_info *cap_info = file->private_data; + if (cap_info->index > 0 && + (cap_info->header.headersize == 0 || + cap_info->count < cap_info->total_size)) { + pr_err("capsule upload not complete\n"); + efi_free_all_buff_pages(cap_info); + } + kfree(cap_info->pages); kfree(cap_info->phys); kfree(file->private_data); @@ -324,7 +308,6 @@ static const struct file_operations efi_capsule_fops = { .owner = THIS_MODULE, .open = efi_capsule_open, .write = efi_capsule_write, - .flush = efi_capsule_flush, .release = efi_capsule_release, .llseek = no_llseek, }; -- cgit v1.2.3