| Message ID | 20250401130840.72119-1-roger.pau@citrix.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | x86/EFI: prevent write-execute sections | expand |
On 01.04.2025 15:08, Roger Pau Monne wrote: > Hello, > > The following series aim to remove the presence of any write and execute > section in the PE Xen image. This is required to support the NX > compatible flag in the PE header. By the end of the series the > resulting PE image has no relocations that apply to text sections, as > text sections are strictly mapped read-execute only. Xen itself > attempting to apply relocations to text would result in page-faults. > > A smoke test is added to Gitlab to ensure the PE NX support doesn't > regress. > > Only patches 5 and 10 are carried over from v1, the rest are new. > > Thanks, Roger. > > Roger Pau Monne (11): > automation/dockers: add to README how to rebuild all containers > x86/mkreloc: fix obtaining PE image base address > x86/mkreloc: use the string table to get names > x86/mkreloc: print the linear address of relocations to read-only > sections > xen: remove -N from the linker command line > x86/efi: discard .text.header for PE binary > x86/efi: discard multiboot related entry code for PE binary > x86/boot: place trampoline code in a non-execute section > x86/efi: avoid a relocation in efi_arch_post_exit_boot() > x86/efi: do not merge all .init sections > automation/x86: add a xen.efi test with a strict NX OVMF build > > automation/build/README.md | 7 ++ > automation/build/fedora/41-x86_64.dockerfile | 5 ++ > automation/gitlab-ci/test.yaml | 9 +++ > automation/scripts/qemu-smoke-x86-64-efi.sh | 22 +++++- > xen/arch/arm/Makefile | 6 +- > xen/arch/ppc/Makefile | 6 +- > xen/arch/riscv/Makefile | 6 +- > xen/arch/x86/Makefile | 12 +-- > xen/arch/x86/boot/head.S | 3 +- > xen/arch/x86/efi/efi-boot.h | 7 +- > xen/arch/x86/efi/mkreloc.c | 77 +++++++++++++++++--- > xen/arch/x86/xen.lds.S | 20 +++-- > 12 files changed, 138 insertions(+), 42 deletions(-) From titles and diffstat (all Makefile changes being covered by patch 05) it looks like you still don't add passing --nxcompat to the linker. Is that intentionally left out here? Jan
On Tue, Apr 01, 2025 at 03:13:52PM +0200, Jan Beulich wrote: > On 01.04.2025 15:08, Roger Pau Monne wrote: > > Hello, > > > > The following series aim to remove the presence of any write and execute > > section in the PE Xen image. This is required to support the NX > > compatible flag in the PE header. By the end of the series the > > resulting PE image has no relocations that apply to text sections, as > > text sections are strictly mapped read-execute only. Xen itself > > attempting to apply relocations to text would result in page-faults. > > > > A smoke test is added to Gitlab to ensure the PE NX support doesn't > > regress. > > > > Only patches 5 and 10 are carried over from v1, the rest are new. > > > > Thanks, Roger. > > > > Roger Pau Monne (11): > > automation/dockers: add to README how to rebuild all containers > > x86/mkreloc: fix obtaining PE image base address > > x86/mkreloc: use the string table to get names > > x86/mkreloc: print the linear address of relocations to read-only > > sections > > xen: remove -N from the linker command line > > x86/efi: discard .text.header for PE binary > > x86/efi: discard multiboot related entry code for PE binary > > x86/boot: place trampoline code in a non-execute section > > x86/efi: avoid a relocation in efi_arch_post_exit_boot() > > x86/efi: do not merge all .init sections > > automation/x86: add a xen.efi test with a strict NX OVMF build > > > > automation/build/README.md | 7 ++ > > automation/build/fedora/41-x86_64.dockerfile | 5 ++ > > automation/gitlab-ci/test.yaml | 9 +++ > > automation/scripts/qemu-smoke-x86-64-efi.sh | 22 +++++- > > xen/arch/arm/Makefile | 6 +- > > xen/arch/ppc/Makefile | 6 +- > > xen/arch/riscv/Makefile | 6 +- > > xen/arch/x86/Makefile | 12 +-- > > xen/arch/x86/boot/head.S | 3 +- > > xen/arch/x86/efi/efi-boot.h | 7 +- > > xen/arch/x86/efi/mkreloc.c | 77 +++++++++++++++++--- > > xen/arch/x86/xen.lds.S | 20 +++-- > > 12 files changed, 138 insertions(+), 42 deletions(-) > > From titles and diffstat (all Makefile changes being covered by patch 05) > it looks like you still don't add passing --nxcompat to the linker. Is > that intentionally left out here? Hm, and I see I also failed to add (the already RB patch) "xen/build: warn about RWX load segments". nxcompat should be enabled by default I think? I can of course make it explicit by adding to the PE link command line. Thanks, Roger.
On 01.04.2025 15:26, Roger Pau Monné wrote: > On Tue, Apr 01, 2025 at 03:13:52PM +0200, Jan Beulich wrote: >> On 01.04.2025 15:08, Roger Pau Monne wrote: >>> Hello, >>> >>> The following series aim to remove the presence of any write and execute >>> section in the PE Xen image. This is required to support the NX >>> compatible flag in the PE header. By the end of the series the >>> resulting PE image has no relocations that apply to text sections, as >>> text sections are strictly mapped read-execute only. Xen itself >>> attempting to apply relocations to text would result in page-faults. >>> >>> A smoke test is added to Gitlab to ensure the PE NX support doesn't >>> regress. >>> >>> Only patches 5 and 10 are carried over from v1, the rest are new. >>> >>> Thanks, Roger. >>> >>> Roger Pau Monne (11): >>> automation/dockers: add to README how to rebuild all containers >>> x86/mkreloc: fix obtaining PE image base address >>> x86/mkreloc: use the string table to get names >>> x86/mkreloc: print the linear address of relocations to read-only >>> sections >>> xen: remove -N from the linker command line >>> x86/efi: discard .text.header for PE binary >>> x86/efi: discard multiboot related entry code for PE binary >>> x86/boot: place trampoline code in a non-execute section >>> x86/efi: avoid a relocation in efi_arch_post_exit_boot() >>> x86/efi: do not merge all .init sections >>> automation/x86: add a xen.efi test with a strict NX OVMF build >>> >>> automation/build/README.md | 7 ++ >>> automation/build/fedora/41-x86_64.dockerfile | 5 ++ >>> automation/gitlab-ci/test.yaml | 9 +++ >>> automation/scripts/qemu-smoke-x86-64-efi.sh | 22 +++++- >>> xen/arch/arm/Makefile | 6 +- >>> xen/arch/ppc/Makefile | 6 +- >>> xen/arch/riscv/Makefile | 6 +- >>> xen/arch/x86/Makefile | 12 +-- >>> xen/arch/x86/boot/head.S | 3 +- >>> xen/arch/x86/efi/efi-boot.h | 7 +- >>> xen/arch/x86/efi/mkreloc.c | 77 +++++++++++++++++--- >>> xen/arch/x86/xen.lds.S | 20 +++-- >>> 12 files changed, 138 insertions(+), 42 deletions(-) >> >> From titles and diffstat (all Makefile changes being covered by patch 05) >> it looks like you still don't add passing --nxcompat to the linker. Is >> that intentionally left out here? > > Hm, and I see I also failed to add (the already RB patch) "xen/build: > warn about RWX load segments". > > nxcompat should be enabled by default I think? I can of course make > it explicit by adding to the PE link command line. It's not always the default for GNU ld: #define DEFAULT_DLL_CHARACTERISTICS (${cygwin_behavior} ? 0 : \ IMAGE_DLL_CHARACTERISTICS_DYNAMIC_BASE \ | IMAGE_DLL_CHARACTERISTICS_HIGH_ENTROPY_VA \ | IMAGE_DLL_CHARACTERISTICS_NX_COMPAT) And even that I'm not sure is entirely right. I think it goes from the assumption that everything that isn't Cygwin is MinGW. EFI, however, is yet something else. I'm further unconvinced that for any environment the linker may reasonably set this bit without the programmer's consent. But of course that's also a matter of how things are documented - there's a command line option after all to turn off the flag. Plus there's yet another concern I have. Historical knowledge (i.e. may no longer be true) of mine is that the DLL characteristics field is applicable only for binaries which have the IMAGE_FILE_DLL flag set in the respective header field. EFI binaries aren't libraries, though. Otoh GNU ld, judging from source code, apparently doesn't set the flag even for DLLs (or I'm simply failing to spot the respective use of IMAGE_FILE_DLL). Whereas going from observations I find the bit is set in DLLs of the Cygwin installation I have sitting around somewhere. (IOW - I'm confused.) Jan
Hello, The following series aim to remove the presence of any write and execute section in the PE Xen image. This is required to support the NX compatible flag in the PE header. By the end of the series the resulting PE image has no relocations that apply to text sections, as text sections are strictly mapped read-execute only. Xen itself attempting to apply relocations to text would result in page-faults. A smoke test is added to Gitlab to ensure the PE NX support doesn't regress. Only patches 5 and 10 are carried over from v1, the rest are new. Thanks, Roger. Roger Pau Monne (11): automation/dockers: add to README how to rebuild all containers x86/mkreloc: fix obtaining PE image base address x86/mkreloc: use the string table to get names x86/mkreloc: print the linear address of relocations to read-only sections xen: remove -N from the linker command line x86/efi: discard .text.header for PE binary x86/efi: discard multiboot related entry code for PE binary x86/boot: place trampoline code in a non-execute section x86/efi: avoid a relocation in efi_arch_post_exit_boot() x86/efi: do not merge all .init sections automation/x86: add a xen.efi test with a strict NX OVMF build automation/build/README.md | 7 ++ automation/build/fedora/41-x86_64.dockerfile | 5 ++ automation/gitlab-ci/test.yaml | 9 +++ automation/scripts/qemu-smoke-x86-64-efi.sh | 22 +++++- xen/arch/arm/Makefile | 6 +- xen/arch/ppc/Makefile | 6 +- xen/arch/riscv/Makefile | 6 +- xen/arch/x86/Makefile | 12 +-- xen/arch/x86/boot/head.S | 3 +- xen/arch/x86/efi/efi-boot.h | 7 +- xen/arch/x86/efi/mkreloc.c | 77 +++++++++++++++++--- xen/arch/x86/xen.lds.S | 20 +++-- 12 files changed, 138 insertions(+), 42 deletions(-)