From patchwork Fri Aug 18 13:44:07 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 715257 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id DCCD8C71148 for ; Fri, 18 Aug 2023 13:45:52 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1377383AbjHRNpV (ORCPT ); Fri, 18 Aug 2023 09:45:21 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:41642 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1377257AbjHRNou (ORCPT ); Fri, 18 Aug 2023 09:44:50 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3D9F130F5; Fri, 18 Aug 2023 06:44:49 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id D02016687F; Fri, 18 Aug 2023 13:44:48 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 6CCDEC433C8; Fri, 18 Aug 2023 13:44:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1692366288; bh=/TH1+90pH64HBFEuEuDip40MoxEIi2uzjH3CIAJlUhQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=O2aZN6k6ifpVgiyLLLkirZmYOBYHExAVmjiJ3TVbF+e8XpbbiKK6PznjpvMJyuA5N Zm5r1MclWklUgxLW/FKBGKTyYjRFS83V5lSNyFOu+EcyCYScjUPvc2Hh9MLfg0IO/2 dkyVfAlT9diSFnqlPZ9Ei0NId9KmJl61R/qfCLh0qyaxekxe5vpVUforK0Bd6FuCnL VCKHPd+wQi88Nm9KUiKt95ihknGWY5ULPoexeJ/j/H2pqZQys/hKmOshvwa4XpUuNK obbad1N3JZDf6ewcCl0f5AgZy5AMEkctoR/Ed+hG+hv5/5D5b8HYBqZQvutYv1bwE0 qhjudeZmKCR5w== From: Ard Biesheuvel To: linux-efi@vger.kernel.org Cc: linux-kernel@vger.kernel.org, Ard Biesheuvel , Evgeniy Baskov , Borislav Petkov , Dave Hansen , Ingo Molnar , Thomas Gleixner , Peter Jones , Matthew Garrett , Gerd Hoffmann , Kees Cook , "H. Peter Anvin" , =?utf-8?q?Marvin_H=C3=A4user?= Subject: [PATCH 02/17] x86/efi: Disregard setup header of loaded image Date: Fri, 18 Aug 2023 15:44:07 +0200 Message-Id: <20230818134422.380032-3-ardb@kernel.org> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20230818134422.380032-1-ardb@kernel.org> References: <20230818134422.380032-1-ardb@kernel.org> MIME-Version: 1.0 X-Developer-Signature: v=1; a=openpgp-sha256; l=4519; i=ardb@kernel.org; h=from:subject; bh=/TH1+90pH64HBFEuEuDip40MoxEIi2uzjH3CIAJlUhQ=; b=owGbwMvMwCFmkMcZplerG8N4Wi2JIeV+6YyeHP+cPzu+P58+z2umrYmJWZXbmctPdT74KHnLL yjkFZ/bUcrCIMbBICumyCIw+++7nacnStU6z5KFmcPKBDKEgYtTACZSL8vIsNw/WP7rpb1brZeE NAVFzdHOz16XEsxQznA1NiZVPsxfh5HhsyDbLAajXe9Yzk07V5FSvNt2v7Jtb/BOx/bfU1mkKuZ xAgA= X-Developer-Key: i=ardb@kernel.org; a=openpgp; fpr=F43D03328115A198C90016883D200E9CA6329909 Precedence: bulk List-ID: X-Mailing-List: linux-efi@vger.kernel.org The native EFI entrypoint does not take a struct boot_params from the loader, but instead, it constructs one from scratch, using the setup header data placed at the start of the image. This setup header is placed in a way that permits legacy loaders to manipulate the contents (i.e., to pass the kernel command line or the address and size of an initial ramdisk), but EFI boot does not use it in that way - it only copies the contents that were placed there at build time, but EFI loaders will not (and should not) manipulate the setup header to configure the boot. (Commit 63bf28ceb3ebbe76 "efi: x86: Wipe setup_data on pure EFI boot" deals with some of the fallout of using setup_data in a way that breaks EFI boot.) As it turns out, there is another reason why copying this header is slightly problematic: it is placed at a fixed offset of 0x1f1 bytes into the image, and this means the PE/COFF view of the image payload has to start there as the setup header is consumed by the program and therefore part of the payload, and this leaves little space to describe all the sections of the image. The only reason to describe the setup header is that the EFI stub may copy this setup header at boot time, but beyond that, none of the contents of this header section are needed by the running code. If the setup header does not need to be accessed at runtime, there is no need to describe it in the first place, freeing up a slot in the section header array. And actually, none of the non-zero values that are copied from the setup header into the EFI stub's struct boot_params are relevant to the boot now that the EFI stub no longer enters via the legacy decompressor. So omit the copy altogether. Signed-off-by: Ard Biesheuvel --- drivers/firmware/efi/libstub/x86-stub.c | 46 +++----------------- 1 file changed, 6 insertions(+), 40 deletions(-) diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c index e976288728e975f1..d35cc47d195debcf 100644 --- a/drivers/firmware/efi/libstub/x86-stub.c +++ b/drivers/firmware/efi/libstub/x86-stub.c @@ -449,9 +449,8 @@ void __noreturn efi_stub_entry(efi_handle_t handle, efi_status_t __efiapi efi_pe_entry(efi_handle_t handle, efi_system_table_t *sys_table_arg) { - struct boot_params *boot_params; - struct setup_header *hdr; - void *image_base; + static struct boot_params boot_params __page_aligned_bss; + struct setup_header *hdr = &boot_params.hdr; efi_guid_t proto = LOADED_IMAGE_PROTOCOL_GUID; int options_size = 0; efi_status_t status; @@ -469,30 +468,9 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle, efi_exit(handle, status); } - image_base = efi_table_attr(image, image_base); - - status = efi_allocate_pages(sizeof(struct boot_params), - (unsigned long *)&boot_params, ULONG_MAX); - if (status != EFI_SUCCESS) { - efi_err("Failed to allocate lowmem for boot params\n"); - efi_exit(handle, status); - } - - memset(boot_params, 0x0, sizeof(struct boot_params)); - - hdr = &boot_params->hdr; - - /* Copy the setup header from the second sector to boot_params */ - memcpy(&hdr->jump, image_base + 512, - sizeof(struct setup_header) - offsetof(struct setup_header, jump)); - - /* - * Fill out some of the header fields ourselves because the - * EFI firmware loader doesn't load the first sector. - */ + /* assign the setup_header fields that the kernel actually cares about */ hdr->root_flags = 1; hdr->vid_mode = 0xffff; - hdr->boot_flag = 0xAA55; hdr->type_of_loader = 0x21; @@ -501,25 +479,13 @@ efi_status_t __efiapi efi_pe_entry(efi_handle_t handle, if (!cmdline_ptr) goto fail; - efi_set_u64_split((unsigned long)cmdline_ptr, - &hdr->cmd_line_ptr, &boot_params->ext_cmd_line_ptr); + efi_set_u64_split((unsigned long)cmdline_ptr, &hdr->cmd_line_ptr, + &boot_params.ext_cmd_line_ptr); - hdr->ramdisk_image = 0; - hdr->ramdisk_size = 0; - - /* - * Disregard any setup data that was provided by the bootloader: - * setup_data could be pointing anywhere, and we have no way of - * authenticating or validating the payload. - */ - hdr->setup_data = 0; - - efi_stub_entry(handle, sys_table_arg, boot_params); + efi_stub_entry(handle, sys_table_arg, &boot_params); /* not reached */ fail: - efi_free(sizeof(struct boot_params), (unsigned long)boot_params); - efi_exit(handle, status); }