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: Sun, 12 Mar 2023 15:01:53 +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-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) >> Also, why .reloc is at offset 0x2000 and not just 0x1000, is there >> anything important I am missing? I understand that is cannot be 0 >> and should be aligned on page size, but nothing else comes to my >> mind... >> > > That was just arbitrary, because the raw allocations of reloc and > compat are also allocated towards the end. But I guess starting at > 0x1000 for .reloc makes more sense so feel free to change that. Thanks for clarifications!
Powered by blists - more mailing lists