From mboxrd@z Thu Jan 1 00:00:00 1970 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 2C2B6C6FD1C for ; Sat, 11 Mar 2023 15:09:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230002AbjCKPJk (ORCPT ); Sat, 11 Mar 2023 10:09:40 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:38406 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229844AbjCKPJh (ORCPT ); Sat, 11 Mar 2023 10:09:37 -0500 Received: from mail.ispras.ru (mail.ispras.ru [83.149.199.84]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 77EB1524A; Sat, 11 Mar 2023 07:09:35 -0800 (PST) Received: from mail.ispras.ru (unknown [83.149.199.84]) by mail.ispras.ru (Postfix) with ESMTPSA id A574C40737CF; Sat, 11 Mar 2023 15:09:33 +0000 (UTC) DKIM-Filter: OpenDKIM Filter v2.11.0 mail.ispras.ru A574C40737CF DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ispras.ru; s=default; t=1678547373; bh=zDtdDisuOVpo+LVgUx80N2IrP23n/FpVuA+p57hry2g=; h=Date:From:To:Cc:Subject:In-Reply-To:References:From; b=dQ169L29RkU5TME2eWOgSPKOUnwedTQAze/wANT0pfmGXx+NgY3ZD0YqqroPbBfNH 1L4781Vbyc6vDgvCMMU4FhzjEfXH8dbwMhnY+j+O8WIvu4chArXK2epcRlO1fQ1oXY lVdGnmQnGqX1hFaWiiCXiWNhdfn85+jMlkjMpZH4= MIME-Version: 1.0 Date: Sat, 11 Mar 2023 18:09:33 +0300 From: Evgeniy Baskov To: Ard Biesheuvel Cc: Borislav Petkov , Andy Lutomirski , Dave Hansen , Ingo Molnar , Peter Zijlstra , Thomas Gleixner , Alexey Khoroshilov , Peter Jones , "Limonciello, Mario" , joeyli , lvc-project@linuxtesting.org, x86@kernel.org, linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org Subject: Re: [PATCH v4 21/26] efi/x86: Explicitly set sections memory attributes In-Reply-To: References: User-Agent: Roundcube Webmail/1.4.4 Message-ID: X-Sender: baskov@ispras.ru Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2023-03-10 18:20, Ard Biesheuvel wrote: > On Thu, 15 Dec 2022 at 13:42, Evgeniy Baskov wrote: >> >> Explicitly change sections memory attributes in efi_pe_entry in case >> of incorrect EFI implementations and to reduce access rights to >> compressed kernel blob. By default it is set executable due to >> restriction in maximum number of sections that can fit before zero >> page. >> >> Tested-by: Peter Jones >> Signed-off-by: Evgeniy Baskov > > I don't think we need this patch. Firmware that cares about W^X will > map the PE image with R-X for text/rodata and RW- for data/bss, which > is sufficient, and firmware that doesn't is a lost cause anyway. This patch were here mainly here to make .rodata non-executable and for the UEFI handover protocol, for which attributes are usually not getting applied. Since the UEFI handover protocol is deprecated, I'll exclude patches from v5 and maybe submit it separately modified to apply attributes only when booting via this protocol. > > >> --- >> drivers/firmware/efi/libstub/x86-stub.c | 54 >> +++++++++++++++++++++++++ >> 1 file changed, 54 insertions(+) >> >> diff --git a/drivers/firmware/efi/libstub/x86-stub.c >> b/drivers/firmware/efi/libstub/x86-stub.c >> index 1f0a2e7075c3..60697fcd8950 100644 >> --- a/drivers/firmware/efi/libstub/x86-stub.c >> +++ b/drivers/firmware/efi/libstub/x86-stub.c >> @@ -27,6 +27,12 @@ const efi_dxe_services_table_t *efi_dxe_table; >> u32 image_offset __section(".data"); >> static efi_loaded_image_t *image __section(".data"); >> >> +extern char _head[], _ehead[]; >> +extern char _compressed[], _ecompressed[]; >> +extern char _text[], _etext[]; >> +extern char _rodata[], _erodata[]; >> +extern char _data[]; >> + >> static efi_status_t >> preserve_pci_rom_image(efi_pci_io_protocol_t *pci, struct >> pci_setup_rom **__rom) >> { >> @@ -343,6 +349,52 @@ void __noreturn efi_exit(efi_handle_t handle, >> efi_status_t status) >> asm("hlt"); >> } >> >> + >> +/* >> + * Manually setup memory protection attributes for each ELF section >> + * since we cannot do it properly by using PE sections. >> + */ >> +static void setup_sections_memory_protection(unsigned long >> image_base) >> +{ >> +#ifdef CONFIG_EFI_DXE_MEM_ATTRIBUTES >> + efi_dxe_table = >> get_efi_config_table(EFI_DXE_SERVICES_TABLE_GUID); >> + >> + if (!efi_dxe_table || >> + efi_dxe_table->hdr.signature != >> EFI_DXE_SERVICES_TABLE_SIGNATURE) { >> + efi_warn("Unable to locate EFI DXE services table\n"); >> + efi_dxe_table = NULL; >> + return; >> + } >> + >> + /* .setup [image_base, _head] */ >> + efi_adjust_memory_range_protection(image_base, >> + (unsigned long)_head - >> image_base, >> + EFI_MEMORY_RO | >> EFI_MEMORY_XP); >> + /* .head.text [_head, _ehead] */ >> + efi_adjust_memory_range_protection((unsigned long)_head, >> + (unsigned long)_ehead - >> (unsigned long)_head, >> + EFI_MEMORY_RO); >> + /* .rodata..compressed [_compressed, _ecompressed] */ >> + efi_adjust_memory_range_protection((unsigned long)_compressed, >> + (unsigned long)_ecompressed >> - (unsigned long)_compressed, >> + EFI_MEMORY_RO | >> EFI_MEMORY_XP); >> + /* .text [_text, _etext] */ >> + efi_adjust_memory_range_protection((unsigned long)_text, >> + (unsigned long)_etext - >> (unsigned long)_text, >> + EFI_MEMORY_RO); >> + /* .rodata [_rodata, _erodata] */ >> + efi_adjust_memory_range_protection((unsigned long)_rodata, >> + (unsigned long)_erodata - >> (unsigned long)_rodata, >> + EFI_MEMORY_RO | >> EFI_MEMORY_XP); >> + /* .data, .bss [_data, _end] */ >> + efi_adjust_memory_range_protection((unsigned long)_data, >> + (unsigned long)_end - >> (unsigned long)_data, >> + EFI_MEMORY_XP); >> +#else >> + (void)image_base; >> +#endif >> +} >> + >> void __noreturn efi_stub_entry(efi_handle_t handle, >> efi_system_table_t *sys_table_arg, >> struct boot_params *boot_params); >> @@ -687,6 +739,8 @@ asmlinkage unsigned long efi_main(efi_handle_t >> handle, >> efi_dxe_table = NULL; >> } >> >> + setup_sections_memory_protection(bzimage_addr - image_offset); >> + >> #ifdef CONFIG_CMDLINE_BOOL >> status = efi_parse_options(CONFIG_CMDLINE); >> if (status != EFI_SUCCESS) { >> -- >> 2.37.4 >>