lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 13 Mar 2023 12:11:09 +0300 From: Evgeniy Baskov <baskov@...ras.ru> To: Ard Biesheuvel <ardb@...nel.org> Cc: Borislav Petkov <bp@...en8.de>, Andy Lutomirski <luto@...nel.org>, Dave Hansen <dave.hansen@...ux.intel.com>, Ingo Molnar <mingo@...hat.com>, Peter Zijlstra <peterz@...radead.org>, Thomas Gleixner <tglx@...utronix.de>, Alexey Khoroshilov <khoroshilov@...ras.ru>, Peter Jones <pjones@...hat.com>, "Limonciello, Mario" <mario.limonciello@....com>, joeyli <jlee@...e.com>, lvc-project@...uxtesting.org, x86@...nel.org, linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org Subject: Re: [PATCH v4 20/26] x86/build: Make generated PE more spec compliant On 2023-03-12 16:09, Ard Biesheuvel wrote: > On Sun, 12 Mar 2023 at 13:02, Evgeniy Baskov <baskov@...ras.ru> wrote: >> >> On 2023-03-11 20:31, Ard Biesheuvel wrote: >> > On Sat, 11 Mar 2023 at 16:02, Evgeniy Baskov <baskov@...ras.ru> wrote: >> >> >> >> On 2023-03-10 18:17, Ard Biesheuvel wrote: >> >> > On Thu, 15 Dec 2022 at 13:42, Evgeniy Baskov <baskov@...ras.ru> wrote: >> >> >> >> >> >> Currently kernel image is not fully compliant PE image, so it may >> >> >> fail to boot with stricter implementations of UEFI PE loaders. >> >> >> >> >> >> Set minimal alignments and sizes specified by PE documentation [1] >> >> >> referenced by UEFI specification [2]. Align PE header to 8 bytes. >> >> >> >> >> >> Generate PE sections dynamically. This simplifies code, since with >> >> >> current implementation all of the sections needs to be defined in >> >> >> header.S, where most section header fields do not hold valid values, >> >> >> except for their names. Before the change, it also held flags, >> >> >> but now flags depend on kernel configuration and it is simpler >> >> >> to set them from build.c too. >> >> >> >> >> >> Setup sections protection. Since we cannot fit every needed section, >> >> >> set a part of protection flags dynamically during initialization. >> >> >> This step is omitted if CONFIG_EFI_DXE_MEM_ATTRIBUTES is not set. >> >> >> >> >> >> [1] >> >> >> https://download.microsoft.com/download/9/c/5/9c5b2167-8017-4bae-9fde-d599bac8184a/pecoff_v83.docx >> >> >> [2] >> >> >> https://uefi.org/sites/default/files/resources/UEFI_Spec_2_9_2021_03_18.pdf >> >> >> >> >> >> Tested-by: Peter Jones <pjones@...hat.com> >> >> >> Signed-off-by: Evgeniy Baskov <baskov@...ras.ru> >> >> > >> >> > I would prefer it if we didn't rewrite the build tool this way. >> >> > >> >> > Having the sections in header.S in the order they appear in the binary >> >> > is rather useful, and I don't think we should manipulate the section >> >> > flags based on whether CONFIG_DXE_MEM_ATTRIBUTES is set. I also don't >> >> > think we need more than .text / .,data (as discussed in the other >> >> > thread on linux-efi@) >> >> > >> >> > Furthermore, I had a look at the audk PE loader [0], and I think it is >> >> > being overly pedantic. >> >> > >> >> > The PE/COFF spec does not require that all sections are virtually >> >> > contiguous, and it does not require that the file content is >> >> > completely covered by either the header or by a section. >> >> > >> >> > So what I would prefer to do is the following: >> >> > >> >> > Sections: >> >> > Idx Name Size VMA Type >> >> > 0 .reloc 00000200 0000000000002000 DATA >> >> > 1 .compat 00000200 0000000000003000 DATA >> >> > 2 .text 00bee000 0000000000004000 TEXT >> >> > 3 .data 00002200 0000000000bf2000 DATA >> >> > >> >> > using 4k section alignment and 512 byte file alignment, and a header >> >> > size of 0x200 as before (This requires my patch that allows the setup >> >> > header to remain unmapped when running the stub [1]) >> >> > >> >> > The reloc and compat payloads are placed at the end of the setup data >> >> > as before, but increased in size to 512 bytes each, and then mapped >> >> > non-1:1 into the RVA space. >> >> > >> >> > This works happily with both the existing PE loader as well as the >> >> > audk one, but with the pedantic flags disabled. >> >> > >> >> >> >> This makes sense. I'll change this patch to use this layout and >> >> to keep sections in headers.S before sending v5. (and I guess I'll >> >> make the compressed kernel a part of .text). I have a few questions >> >> though: >> >> >> >> This layout assumes having the local copy of the bootparams as >> >> in your RFC patches, right? >> >> >> > >> > Indeed. Otherwise, the setup header may not have been copied to memory >> > by the loader. >> > >> >> Can I keep the .rodata -- 5th section fits in the section table >> >> without much work? >> >> >> > >> > You could, but at least the current PE/COFF loader in EDK2 will map it >> > read/write, as it only distinguishes between executable sections and >> > non-executable sections. >> > >> >> At least it will slightly improve security for some implementations >> (e.g. audk, while being overly strict support RO sections) >> > > Yeah, but more common loaders will put the compressed data in a > writable region. I'd prefer to have a simple and common baseline where > we always just use R-X for all text and rodata, and RW- for everything > else. Hmm... I'll remove the .rodata for now then. If anything changes I can always submit it as a separate patch later anyways.
Powered by blists - more mailing lists