[PATCH v3 0/4] Various patches to improve Secure Boot support

[PATCH v3 0/4] Various patches to improve Secure Boot support

[Frediano Ziglio]

These patches improve support for Secure boot.
UEFI CA memory mitigation requires memory pages to be not executable and
writable at the same time. So changing permissions and splitting some section
is required.
Remove multiboot pieces from EFI executable.

Changes since v1:
- improved some comments;
- merged 2 pacthes removing multiboot support in x86 PE;
- removed a patch dealing with SBAT;
- other minor changes (see single patches).

Changes since v2:
- improved some comments.

Frediano Ziglio (2):
  Align relevant sections to 4KB
  x86: Split .init section to satisfy UEFI CA memory mitigation

Roger Pau Monné (2):
  x86/efi: discard multiboot support for PE binary
  x86/efi: avoid a relocation in efi_arch_post_exit_boot()

 docs/hypervisor-guide/x86/how-xen-boots.rst |  6 ------
 xen/arch/x86/boot/head.S                    |  3 ++-
 xen/arch/x86/efi/efi-boot.h                 |  7 +++++--
 xen/arch/x86/xen.lds.S                      | 22 +++++++++++----------
 4 files changed, 19 insertions(+), 19 deletions(-)

-- 
2.43.0

[PATCH v3 1/4] Align relevant sections to 4KB

[Frediano Ziglio]

From: Frediano Ziglio <frediano.ziglio@cloud.com>

Required by UEFI CA memory mitigation.

It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
in the pagetables.

NX_COMPAT is a requirement from shim-review,
https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility

Sections with different permissions must be in separate pages.
In the case of debug sections they are contiguous and have the same
permissions so it's not an issue if they are not aligned to the page.

Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
--
Changes since v1:
- Change subject.

Changes since v2:
- Improved commit message and subject.
---
 xen/arch/x86/xen.lds.S | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index b9e888e596..f758940674 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -162,8 +162,8 @@ SECTIONS
        __note_gnu_build_id_end = .;
   } PHDR(note) PHDR(text)
 #elif defined(BUILD_ID_EFI)
-  /* Workaround bug in binutils < 2.36 */
-  . = ALIGN(32);
+  /* Align to satisfy UEFI CA memory mitigation. */
+  . = ALIGN(PAGE_SIZE);
   DECL_SECTION(.buildid) {
        __note_gnu_build_id_start = .;
        *(.buildid)
@@ -330,6 +330,7 @@ SECTIONS
   __2M_rwdata_end = ALIGN(SECTION_ALIGN);
 
 #ifdef EFI
+  . = ALIGN(PAGE_SIZE);
   .reloc ALIGN(4) : {
     __base_relocs_start = .;
     *(.reloc)
-- 
2.43.0

Re: [PATCH v3 1/4] Align relevant sections to 4KB

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 1924 bytes --]

On Tue, Jun 16, 2026 at 11:13:33AM +0100, Frediano Ziglio wrote:
> From: Frediano Ziglio <frediano.ziglio@cloud.com>
> 
> Required by UEFI CA memory mitigation.
> 
> It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
> in the pagetables.
> 
> NX_COMPAT is a requirement from shim-review,
> https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility
> 
> Sections with different permissions must be in separate pages.
> In the case of debug sections they are contiguous and have the same
> permissions so it's not an issue if they are not aligned to the page.
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>

Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

> --
> Changes since v1:
> - Change subject.
> 
> Changes since v2:
> - Improved commit message and subject.
> ---
>  xen/arch/x86/xen.lds.S | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> index b9e888e596..f758940674 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -162,8 +162,8 @@ SECTIONS
>         __note_gnu_build_id_end = .;
>    } PHDR(note) PHDR(text)
>  #elif defined(BUILD_ID_EFI)
> -  /* Workaround bug in binutils < 2.36 */
> -  . = ALIGN(32);
> +  /* Align to satisfy UEFI CA memory mitigation. */
> +  . = ALIGN(PAGE_SIZE);
>    DECL_SECTION(.buildid) {
>         __note_gnu_build_id_start = .;
>         *(.buildid)
> @@ -330,6 +330,7 @@ SECTIONS
>    __2M_rwdata_end = ALIGN(SECTION_ALIGN);
>  
>  #ifdef EFI
> +  . = ALIGN(PAGE_SIZE);
>    .reloc ALIGN(4) : {
>      __base_relocs_start = .;
>      *(.reloc)
> -- 
> 2.43.0
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 1/4] Align relevant sections to 4KB

[Jan Beulich]

On 16.06.2026 12:13, Frediano Ziglio wrote:
> From: Frediano Ziglio <frediano.ziglio@cloud.com>
> 
> Required by UEFI CA memory mitigation.
> 
> It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
> in the pagetables.
> 
> NX_COMPAT is a requirement from shim-review,
> https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility
> 
> Sections with different permissions must be in separate pages.
> In the case of debug sections they are contiguous and have the same
> permissions so it's not an issue if they are not aligned to the page.

What if .debug_* starts in the middle of a page? Aren't you further
relying on .debug_* to be r/o (i.e. neither X nor W)? (Right now
.reloc is what comes immediately ahead of .debug_*, and that's r/o
as well, so not an issue in practice for now. Yet as indicated, the
description here wants to be usable as a reference when this later
needs extending / revisiting.)

Jan

Re: [PATCH v3 1/4] Align relevant sections to 4KB

[Frediano Ziglio]

On Tue, 16 Jun 2026 at 13:27, Jan Beulich <jbeulich@suse.com> wrote:
>
> On 16.06.2026 12:13, Frediano Ziglio wrote:
> > From: Frediano Ziglio <frediano.ziglio@cloud.com>
> >
> > Required by UEFI CA memory mitigation.
> >
> > It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
> > in the pagetables.
> >
> > NX_COMPAT is a requirement from shim-review,
> > https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility
> >
> > Sections with different permissions must be in separate pages.
> > In the case of debug sections they are contiguous and have the same
> > permissions so it's not an issue if they are not aligned to the page.
>
> What if .debug_* starts in the middle of a page? Aren't you further
> relying on .debug_* to be r/o (i.e. neither X nor W)? (Right now
> .reloc is what comes immediately ahead of .debug_*, and that's r/o
> as well, so not an issue in practice for now. Yet as indicated, the
> description here wants to be usable as a reference when this later
> needs extending / revisiting.)
>
> Jan

Can you suggest a better wording?
Practically I think before the .debug section you could have the
.reloc or the SBAT, either are permission-compatible. If in the future
we break it for some reason we'll fix it again.

Frediano

Re: [PATCH v3 1/4] Align relevant sections to 4KB

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 1848 bytes --]

On Tue, Jun 16, 2026 at 03:38:53PM +0100, Frediano Ziglio wrote:
> On Tue, 16 Jun 2026 at 13:27, Jan Beulich <jbeulich@suse.com> wrote:
> >
> > On 16.06.2026 12:13, Frediano Ziglio wrote:
> > > From: Frediano Ziglio <frediano.ziglio@cloud.com>
> > >
> > > Required by UEFI CA memory mitigation.
> > >
> > > It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
> > > in the pagetables.
> > >
> > > NX_COMPAT is a requirement from shim-review,
> > > https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility
> > >
> > > Sections with different permissions must be in separate pages.
> > > In the case of debug sections they are contiguous and have the same
> > > permissions so it's not an issue if they are not aligned to the page.
> >
> > What if .debug_* starts in the middle of a page? Aren't you further
> > relying on .debug_* to be r/o (i.e. neither X nor W)? (Right now
> > .reloc is what comes immediately ahead of .debug_*, and that's r/o
> > as well, so not an issue in practice for now. Yet as indicated, the
> > description here wants to be usable as a reference when this later
> > needs extending / revisiting.)
> >
> > Jan
> 
> Can you suggest a better wording?
> Practically I think before the .debug section you could have the
> .reloc or the SBAT, either are permission-compatible. If in the future
> we break it for some reason we'll fix it again.

Once all of the relevant SB work is upstream, I would definitely want to
have a test in CI for that. We already have a test for booting xen.efi,
extending it to try SB-signed one should not be too hard (famous last
words...).

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 1/4] Align relevant sections to 4KB

[Jan Beulich]

On 16.06.2026 16:38, Frediano Ziglio wrote:
> On Tue, 16 Jun 2026 at 13:27, Jan Beulich <jbeulich@suse.com> wrote:
>>
>> On 16.06.2026 12:13, Frediano Ziglio wrote:
>>> From: Frediano Ziglio <frediano.ziglio@cloud.com>
>>>
>>> Required by UEFI CA memory mitigation.
>>>
>>> It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
>>> in the pagetables.
>>>
>>> NX_COMPAT is a requirement from shim-review,
>>> https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility
>>>
>>> Sections with different permissions must be in separate pages.
>>> In the case of debug sections they are contiguous and have the same
>>> permissions so it's not an issue if they are not aligned to the page.
>>
>> What if .debug_* starts in the middle of a page? Aren't you further
>> relying on .debug_* to be r/o (i.e. neither X nor W)? (Right now
>> .reloc is what comes immediately ahead of .debug_*, and that's r/o
>> as well, so not an issue in practice for now. Yet as indicated, the
>> description here wants to be usable as a reference when this later
>> needs extending / revisiting.)
> 
> Can you suggest a better wording?

After "have the same permissions" insert ", including the immediately
preceding .reloc section,"?

> Practically I think before the .debug section you could have the
> .reloc or the SBAT, either are permission-compatible.

Right, just that this doesn't go without saying.

> If in the future we break it for some reason we'll fix it again.

Yet better would be if we didn't break it.

Jan

[PATCH v3 2/4] x86/efi: discard multiboot support for PE binary

[Frediano Ziglio]

From: Roger Pau Monné <roger.pau@citrix.com>

Multiboot and PVH booting are not supported for PE, hence discards them
in the linker script when doing a PE build.

That removes some relocations that otherwise appear due to the usage of the
start and __efi64_mb2_start symbols in the multiboot2 header.

Section discarding is not done updating DISCARD_SECTIONS definition as the
change is specific for x86.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
--
Changes since v1:
- improve commit message;
- change section orders to avoid changing code order in final executable;
- merge 2 commits;
- removed deprecated documentation section.

Changes since v2:
- Update commit message, join 2 sentences together.
---
 docs/hypervisor-guide/x86/how-xen-boots.rst | 6 ------
 xen/arch/x86/boot/head.S                    | 3 ++-
 xen/arch/x86/xen.lds.S                      | 5 +++++
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/docs/hypervisor-guide/x86/how-xen-boots.rst b/docs/hypervisor-guide/x86/how-xen-boots.rst
index 8b3229005c..b6d852050a 100644
--- a/docs/hypervisor-guide/x86/how-xen-boots.rst
+++ b/docs/hypervisor-guide/x86/how-xen-boots.rst
@@ -82,12 +82,6 @@ When a PEI-capable toolchain is found, the objects are linked together and a
 PE32+ binary is created.  It can be run directly from the EFI shell, and has
 ``efi_start`` as its entry symbol.
 
-.. note::
-
-   xen.efi does contain all MB1/MB2/PVH tags included in the rest of the
-   build.  However, entry via anything other than the EFI64 protocol is
-   unsupported, and won't work.
-
 
 Boot
 ----
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
index 77bb7a9e21..90faf411b9 100644
--- a/xen/arch/x86/boot/head.S
+++ b/xen/arch/x86/boot/head.S
@@ -152,7 +152,7 @@ vga_text_buffer:
 efi_platform:
         .byte   0
 
-        .section .init.text, "ax", @progbits
+        .section .init.multiboot, "ax", @progbits
 
 early_error: /* Here to improve the disassembly. */
 
@@ -710,6 +710,7 @@ trampoline_setup:
         /* Jump into the relocated trampoline. */
         lret
 
+        .section .init.text, "ax", @progbits
 ENTRY(trampoline_start)
 #include "trampoline.S"
 ENTRY(trampoline_end)
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index f758940674..749d9719cc 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -57,6 +57,10 @@ SECTIONS
   __image_base__ = .;
 #else
   . = __image_base__;
+  /DISCARD/ : {
+    *(.text.header)
+    *(.init.multiboot)
+  }
 #endif
 
 #if 0
@@ -195,6 +199,7 @@ SECTIONS
   DECL_SECTION(.init.text) {
 #endif
        _sinittext = .;
+       *(.init.multiboot)
        *(.init.text)
        *(.text.startup)
        _einittext = .;
-- 
2.43.0

Re: [PATCH v3 2/4] x86/efi: discard multiboot support for PE binary

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 3321 bytes --]

On Tue, Jun 16, 2026 at 11:13:34AM +0100, Frediano Ziglio wrote:
> From: Roger Pau Monné <roger.pau@citrix.com>
> 
> Multiboot and PVH booting are not supported for PE, hence discards them
> in the linker script when doing a PE build.
> 
> That removes some relocations that otherwise appear due to the usage of the
> start and __efi64_mb2_start symbols in the multiboot2 header.
> 
> Section discarding is not done updating DISCARD_SECTIONS definition as the
> change is specific for x86.
> 
> No functional change intended.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>

Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

> --
> Changes since v1:
> - improve commit message;
> - change section orders to avoid changing code order in final executable;
> - merge 2 commits;
> - removed deprecated documentation section.
> 
> Changes since v2:
> - Update commit message, join 2 sentences together.
> ---
>  docs/hypervisor-guide/x86/how-xen-boots.rst | 6 ------
>  xen/arch/x86/boot/head.S                    | 3 ++-
>  xen/arch/x86/xen.lds.S                      | 5 +++++
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/docs/hypervisor-guide/x86/how-xen-boots.rst b/docs/hypervisor-guide/x86/how-xen-boots.rst
> index 8b3229005c..b6d852050a 100644
> --- a/docs/hypervisor-guide/x86/how-xen-boots.rst
> +++ b/docs/hypervisor-guide/x86/how-xen-boots.rst
> @@ -82,12 +82,6 @@ When a PEI-capable toolchain is found, the objects are linked together and a
>  PE32+ binary is created.  It can be run directly from the EFI shell, and has
>  ``efi_start`` as its entry symbol.
>  
> -.. note::
> -
> -   xen.efi does contain all MB1/MB2/PVH tags included in the rest of the
> -   build.  However, entry via anything other than the EFI64 protocol is
> -   unsupported, and won't work.
> -
>  
>  Boot
>  ----
> diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S
> index 77bb7a9e21..90faf411b9 100644
> --- a/xen/arch/x86/boot/head.S
> +++ b/xen/arch/x86/boot/head.S
> @@ -152,7 +152,7 @@ vga_text_buffer:
>  efi_platform:
>          .byte   0
>  
> -        .section .init.text, "ax", @progbits
> +        .section .init.multiboot, "ax", @progbits
>  
>  early_error: /* Here to improve the disassembly. */
>  
> @@ -710,6 +710,7 @@ trampoline_setup:
>          /* Jump into the relocated trampoline. */
>          lret
>  
> +        .section .init.text, "ax", @progbits
>  ENTRY(trampoline_start)
>  #include "trampoline.S"
>  ENTRY(trampoline_end)
> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> index f758940674..749d9719cc 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -57,6 +57,10 @@ SECTIONS
>    __image_base__ = .;
>  #else
>    . = __image_base__;
> +  /DISCARD/ : {
> +    *(.text.header)
> +    *(.init.multiboot)
> +  }
>  #endif
>  
>  #if 0
> @@ -195,6 +199,7 @@ SECTIONS
>    DECL_SECTION(.init.text) {
>  #endif
>         _sinittext = .;
> +       *(.init.multiboot)
>         *(.init.text)
>         *(.text.startup)
>         _einittext = .;
> -- 
> 2.43.0
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v3 3/4] x86/efi: avoid a relocation in efi_arch_post_exit_boot()

[Frediano Ziglio]

From: Roger Pau Monné <roger.pau@citrix.com>

Instead of using the absolute __start_xen address, calculate it as an
offset from the current instruction pointer.  The relocation would be
problematic if the generated PE binary had .init.text as a standalone
section with just read and execute permissions."

Removing this relocation is necessary to make it safe to split .init.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
--
Changes since v1:
- Improve commit message.
---
 xen/arch/x86/efi/efi-boot.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
index d738b839ee..b983f054b5 100644
--- a/xen/arch/x86/efi/efi-boot.h
+++ b/xen/arch/x86/efi/efi-boot.h
@@ -270,7 +270,9 @@ static void __init noreturn efi_arch_post_exit_boot(void)
 
                    /* Jump to higher mappings. */
                    "mov    stack_start(%%rip), %%rsp\n\t"
-                   "movabs $__start_xen, %[rip]\n\t"
+                   "lea    __start_xen(%%rip), %[rip]\n\t"
+                   "add    %[offset], %[rip]\n\t"
+
                    "push   %[cs]\n\t"
                    "push   %[rip]\n\t"
                    "lretq"
@@ -278,7 +280,8 @@ static void __init noreturn efi_arch_post_exit_boot(void)
                      [cr4] "+&r" (cr4)
                    : [cr3] "r" (idle_pg_table),
                      [cs] "i" (__HYPERVISOR_CS),
-                     [ds] "r" (__HYPERVISOR_DS)
+                     [ds] "r" (__HYPERVISOR_DS),
+                     [offset] "r" (__XEN_VIRT_START - xen_phys_start)
                    : "memory" );
     unreachable();
 }
-- 
2.43.0

Re: [PATCH v3 3/4] x86/efi: avoid a relocation in efi_arch_post_exit_boot()

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 2120 bytes --]

On Tue, Jun 16, 2026 at 11:13:35AM +0100, Frediano Ziglio wrote:
> From: Roger Pau Monné <roger.pau@citrix.com>
> 
> Instead of using the absolute __start_xen address, calculate it as an
> offset from the current instruction pointer.  The relocation would be
> problematic if the generated PE binary had .init.text as a standalone
> section with just read and execute permissions."
> 
> Removing this relocation is necessary to make it safe to split .init.
> 
> No functional change intended.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>

Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> --
> Changes since v1:
> - Improve commit message.
> ---
>  xen/arch/x86/efi/efi-boot.h | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h
> index d738b839ee..b983f054b5 100644
> --- a/xen/arch/x86/efi/efi-boot.h
> +++ b/xen/arch/x86/efi/efi-boot.h
> @@ -270,7 +270,9 @@ static void __init noreturn efi_arch_post_exit_boot(void)
>  
>                     /* Jump to higher mappings. */
>                     "mov    stack_start(%%rip), %%rsp\n\t"
> -                   "movabs $__start_xen, %[rip]\n\t"
> +                   "lea    __start_xen(%%rip), %[rip]\n\t"
> +                   "add    %[offset], %[rip]\n\t"
> +
>                     "push   %[cs]\n\t"
>                     "push   %[rip]\n\t"
>                     "lretq"
> @@ -278,7 +280,8 @@ static void __init noreturn efi_arch_post_exit_boot(void)
>                       [cr4] "+&r" (cr4)
>                     : [cr3] "r" (idle_pg_table),
>                       [cs] "i" (__HYPERVISOR_CS),
> -                     [ds] "r" (__HYPERVISOR_DS)
> +                     [ds] "r" (__HYPERVISOR_DS),
> +                     [offset] "r" (__XEN_VIRT_START - xen_phys_start)
>                     : "memory" );
>      unreachable();
>  }
> -- 
> 2.43.0
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[PATCH v3 4/4] x86: Split .init section to satisfy UEFI CA memory mitigation

[Frediano Ziglio]

From: Frediano Ziglio <frediano.ziglio@cloud.com>

Currently .init section is both writeable and executable, split data and code
to have 2 sections satisfying W^X rule.

It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
in the pagetables.

NX_COMPAT is a requirement from shim-review,
https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility

Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
--
Change since v1:
- update comment style.
---
 xen/arch/x86/xen.lds.S | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index 749d9719cc..8fefda1816 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -193,11 +193,7 @@ SECTIONS
   __2M_init_start = .;         /* Start of 2M superpages, mapped RWX (boot only). */
   . = ALIGN(PAGE_SIZE);             /* Init code and data */
   __init_begin = .;
-#ifdef EFI /* EFI wants to merge all of .init.*  ELF doesn't. */
-  DECL_SECTION(.init) {
-#else
   DECL_SECTION(.init.text) {
-#endif
        _sinittext = .;
        *(.init.multiboot)
        *(.init.text)
@@ -210,12 +206,12 @@ SECTIONS
         */
        *(.altinstr_replacement)
 
-#ifdef EFI /* EFI wants to merge all of .init.*  ELF doesn't. */
-       . = ALIGN(SMP_CACHE_BYTES);
-#else
   } PHDR(text)
-  DECL_SECTION(.init.data) {
+#ifdef EFI
+  /* Align to satisfy UEFI CA memory mitigation. */
+  . = ALIGN(SECTION_ALIGN);
 #endif
+  DECL_SECTION(.init.data) {
        *(.init.bss.stack_aligned)
        *(.init.data.page_aligned)
 
-- 
2.43.0

Re: [PATCH v3 4/4] x86: Split .init section to satisfy UEFI CA memory mitigation

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 2297 bytes --]

On Tue, Jun 16, 2026 at 11:13:36AM +0100, Frediano Ziglio wrote:
> From: Frediano Ziglio <frediano.ziglio@cloud.com>
> 
> Currently .init section is both writeable and executable, split data and code
> to have 2 sections satisfying W^X rule.
> 
> It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
> in the pagetables.
> 
> NX_COMPAT is a requirement from shim-review,
> https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility
> 
> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>

Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>

Is that the last piece necessary to satisfy the NX_COMPAT requirement? If
so, I suppose a subsequent patch should actually set the
IMAGE_DLLCHARACTERISTICS_NX_COMPAT bit (IIUC ld --nxcompat option), right?

> --
> Change since v1:
> - update comment style.
> ---
>  xen/arch/x86/xen.lds.S | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> index 749d9719cc..8fefda1816 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -193,11 +193,7 @@ SECTIONS
>    __2M_init_start = .;         /* Start of 2M superpages, mapped RWX (boot only). */
>    . = ALIGN(PAGE_SIZE);             /* Init code and data */
>    __init_begin = .;
> -#ifdef EFI /* EFI wants to merge all of .init.*  ELF doesn't. */
> -  DECL_SECTION(.init) {
> -#else
>    DECL_SECTION(.init.text) {
> -#endif
>         _sinittext = .;
>         *(.init.multiboot)
>         *(.init.text)
> @@ -210,12 +206,12 @@ SECTIONS
>          */
>         *(.altinstr_replacement)
>  
> -#ifdef EFI /* EFI wants to merge all of .init.*  ELF doesn't. */
> -       . = ALIGN(SMP_CACHE_BYTES);
> -#else
>    } PHDR(text)
> -  DECL_SECTION(.init.data) {
> +#ifdef EFI
> +  /* Align to satisfy UEFI CA memory mitigation. */
> +  . = ALIGN(SECTION_ALIGN);
>  #endif
> +  DECL_SECTION(.init.data) {
>         *(.init.bss.stack_aligned)
>         *(.init.data.page_aligned)
>  
> -- 
> 2.43.0
> 

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH v3 4/4] x86: Split .init section to satisfy UEFI CA memory mitigation

[Andrew Cooper]

On 16/06/2026 12:20 pm, Marek Marczykowski-Górecki wrote:
> On Tue, Jun 16, 2026 at 11:13:36AM +0100, Frediano Ziglio wrote:
>> From: Frediano Ziglio <frediano.ziglio@cloud.com>
>>
>> Currently .init section is both writeable and executable, split data and code
>> to have 2 sections satisfying W^X rule.
>>
>> It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
>> in the pagetables.
>>
>> NX_COMPAT is a requirement from shim-review,
>> https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility
>>
>> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
> Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
> Is that the last piece necessary to satisfy the NX_COMPAT requirement? If
> so, I suppose a subsequent patch should actually set the
> IMAGE_DLLCHARACTERISTICS_NX_COMPAT bit (IIUC ld --nxcompat option), right?

We need to satisfy everything in
https://github.com/xenserver/xen.pg/blob/XS-9/patches/correct-sections-permissions.patch

.reloc needs to be non-discardable and writeable.  This will require a
very recent binutils and a patch (series?) from Jan.

~Andrew

Re: [PATCH v3 4/4] x86: Split .init section to satisfy UEFI CA memory mitigation

[Jan Beulich]

On 16.06.2026 13:29, Andrew Cooper wrote:
> On 16/06/2026 12:20 pm, Marek Marczykowski-Górecki wrote:
>> On Tue, Jun 16, 2026 at 11:13:36AM +0100, Frediano Ziglio wrote:
>>> From: Frediano Ziglio <frediano.ziglio@cloud.com>
>>>
>>> Currently .init section is both writeable and executable, split data and code
>>> to have 2 sections satisfying W^X rule.
>>>
>>> It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
>>> in the pagetables.
>>>
>>> NX_COMPAT is a requirement from shim-review,
>>> https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility
>>>
>>> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
>> Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>>
>> Is that the last piece necessary to satisfy the NX_COMPAT requirement? If
>> so, I suppose a subsequent patch should actually set the
>> IMAGE_DLLCHARACTERISTICS_NX_COMPAT bit (IIUC ld --nxcompat option), right?
> 
> We need to satisfy everything in
> https://github.com/xenserver/xen.pg/blob/XS-9/patches/correct-sections-permissions.patch
> 
> .reloc needs to be non-discardable and writeable.

Writable? Why?

>  This will require a very recent binutils and a patch (series?) from Jan.

Seeing that no patch had been submitted so far, I recently added this to my
todo list, yes. But really I was hoping that someone else would make the
small change that I expect is going to be needed.

Jan

Re: [PATCH v3 4/4] x86: Split .init section to satisfy UEFI CA memory mitigation

[Andrew Cooper]

On 16/06/2026 1:30 pm, Jan Beulich wrote:
> On 16.06.2026 13:29, Andrew Cooper wrote:
>> On 16/06/2026 12:20 pm, Marek Marczykowski-Górecki wrote:
>>> On Tue, Jun 16, 2026 at 11:13:36AM +0100, Frediano Ziglio wrote:
>>>> From: Frediano Ziglio <frediano.ziglio@cloud.com>
>>>>
>>>> Currently .init section is both writeable and executable, split data and code
>>>> to have 2 sections satisfying W^X rule.
>>>>
>>>> It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
>>>> in the pagetables.
>>>>
>>>> NX_COMPAT is a requirement from shim-review,
>>>> https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility
>>>>
>>>> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
>>> Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>>>
>>> Is that the last piece necessary to satisfy the NX_COMPAT requirement? If
>>> so, I suppose a subsequent patch should actually set the
>>> IMAGE_DLLCHARACTERISTICS_NX_COMPAT bit (IIUC ld --nxcompat option), right?
>> We need to satisfy everything in
>> https://github.com/xenserver/xen.pg/blob/XS-9/patches/correct-sections-permissions.patch
>>
>> .reloc needs to be non-discardable and writeable.
> Writable? Why?

Because we take fatal pagefaults against it when it's really read-only.

But as for why, I'll have to defer that to Frediano/Ross who did the
work originally.

>
>>   This will require a very recent binutils and a patch (series?) from Jan.
> Seeing that no patch had been submitted so far, I recently added this to my
> todo list, yes. But really I was hoping that someone else would make the
> small change that I expect is going to be needed.

If you can explain what change is needed then maybe someone else can do it.

But right now, all I know is it's a new binutils and "something".

~Andrew

Re: [PATCH v3 4/4] x86: Split .init section to satisfy UEFI CA memory mitigation

[Frediano Ziglio]

On Tue, 16 Jun 2026 at 13:40, Andrew Cooper <andrew.cooper3@citrix.com> wrote:
>
> On 16/06/2026 1:30 pm, Jan Beulich wrote:
> > On 16.06.2026 13:29, Andrew Cooper wrote:
> >> On 16/06/2026 12:20 pm, Marek Marczykowski-Górecki wrote:
> >>> On Tue, Jun 16, 2026 at 11:13:36AM +0100, Frediano Ziglio wrote:
> >>>> From: Frediano Ziglio <frediano.ziglio@cloud.com>
> >>>>
> >>>> Currently .init section is both writeable and executable, split data and code
> >>>> to have 2 sections satisfying W^X rule.
> >>>>
> >>>> It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
> >>>> in the pagetables.
> >>>>
> >>>> NX_COMPAT is a requirement from shim-review,
> >>>> https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility
> >>>>
> >>>> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
> >>> Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> >>>
> >>> Is that the last piece necessary to satisfy the NX_COMPAT requirement? If
> >>> so, I suppose a subsequent patch should actually set the
> >>> IMAGE_DLLCHARACTERISTICS_NX_COMPAT bit (IIUC ld --nxcompat option), right?

At the moment we don't have downstream patches changing the nxcompat
specifically. Recent toolchains combinations are setting that flag
automatically.

> >> We need to satisfy everything in
> >> https://github.com/xenserver/xen.pg/blob/XS-9/patches/correct-sections-permissions.patch
> >>
> >> .reloc needs to be non-discardable and writeable.
> > Writable? Why?
>
> Because we take fatal pagefaults against it when it's really read-only.
>
> But as for why, I'll have to defer that to Frediano/Ross who did the
> work originally.
>

Wait, there's a bit of confusion. The .reloc section needs to be
non-discardable but read-only is fine, it's the .rodata section that
has to be writable. The reason is that some variables are "read
mostly" or "writable during init". This is consistent with what the
script is doing.

> >
> >>   This will require a very recent binutils and a patch (series?) from Jan.
> > Seeing that no patch had been submitted so far, I recently added this to my
> > todo list, yes. But really I was hoping that someone else would make the
> > small change that I expect is going to be needed.
>
> If you can explain what change is needed then maybe someone else can do it.
>
> But right now, all I know is it's a new binutils and "something".
>
> ~Andrew

The patch was not submitted as similar patches were rejected as too
"hacky" and instead was suggested to have binutils changes. On the
other hand, former binutils must continue to "work" even if they
create wrong output. So instead of having to wait 5/6 years so that
all Xen supported binutils have the features we need we have that
patch in our series.

Frediano

Re: [PATCH v3 4/4] x86: Split .init section to satisfy UEFI CA memory mitigation

[Jan Beulich]

On 16.06.2026 14:40, Andrew Cooper wrote:
> On 16/06/2026 1:30 pm, Jan Beulich wrote:
>> On 16.06.2026 13:29, Andrew Cooper wrote:
>>> On 16/06/2026 12:20 pm, Marek Marczykowski-Górecki wrote:
>>>> On Tue, Jun 16, 2026 at 11:13:36AM +0100, Frediano Ziglio wrote:
>>>>> From: Frediano Ziglio <frediano.ziglio@cloud.com>
>>>>>
>>>>> Currently .init section is both writeable and executable, split data and code
>>>>> to have 2 sections satisfying W^X rule.
>>>>>
>>>>> It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
>>>>> in the pagetables.
>>>>>
>>>>> NX_COMPAT is a requirement from shim-review,
>>>>> https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility
>>>>>
>>>>> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
>>>> Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>>>>
>>>> Is that the last piece necessary to satisfy the NX_COMPAT requirement? If
>>>> so, I suppose a subsequent patch should actually set the
>>>> IMAGE_DLLCHARACTERISTICS_NX_COMPAT bit (IIUC ld --nxcompat option), right?
>>> We need to satisfy everything in
>>> https://github.com/xenserver/xen.pg/blob/XS-9/patches/correct-sections-permissions.patch
>>>
>>> .reloc needs to be non-discardable and writeable.
>> Writable? Why?
> 
> Because we take fatal pagefaults against it when it's really read-only.

Wasn't this for relocations _against_ r/o sections, not the .reloc section
itself?

Jan

Re: [PATCH v3 4/4] x86: Split .init section to satisfy UEFI CA memory mitigation

[Andrew Cooper]

On 16/06/2026 2:50 pm, Jan Beulich wrote:
> On 16.06.2026 14:40, Andrew Cooper wrote:
>> On 16/06/2026 1:30 pm, Jan Beulich wrote:
>>> On 16.06.2026 13:29, Andrew Cooper wrote:
>>>> On 16/06/2026 12:20 pm, Marek Marczykowski-Górecki wrote:
>>>>> On Tue, Jun 16, 2026 at 11:13:36AM +0100, Frediano Ziglio wrote:
>>>>>> From: Frediano Ziglio <frediano.ziglio@cloud.com>
>>>>>>
>>>>>> Currently .init section is both writeable and executable, split data and code
>>>>>> to have 2 sections satisfying W^X rule.
>>>>>>
>>>>>> It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
>>>>>> in the pagetables.
>>>>>>
>>>>>> NX_COMPAT is a requirement from shim-review,
>>>>>> https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility
>>>>>>
>>>>>> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
>>>>> Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>>>>>
>>>>> Is that the last piece necessary to satisfy the NX_COMPAT requirement? If
>>>>> so, I suppose a subsequent patch should actually set the
>>>>> IMAGE_DLLCHARACTERISTICS_NX_COMPAT bit (IIUC ld --nxcompat option), right?
>>>> We need to satisfy everything in
>>>> https://github.com/xenserver/xen.pg/blob/XS-9/patches/correct-sections-permissions.patch
>>>>
>>>> .reloc needs to be non-discardable and writeable.
>>> Writable? Why?
>> Because we take fatal pagefaults against it when it's really read-only.
> Wasn't this for relocations _against_ r/o sections, not the .reloc section
> itself?

Frediano pointed out that I read the script incorrectly.

.reloc needs to be non-discardable

.rodata needs to be writeable because of editing the relocations in it.

~Andrew

Re: [PATCH v3 4/4] x86: Split .init section to satisfy UEFI CA memory mitigation

[Andrew Cooper]

On 16/06/2026 12:20 pm, Marek Marczykowski-Górecki wrote:
> On Tue, Jun 16, 2026 at 11:13:36AM +0100, Frediano Ziglio wrote:
>> From: Frediano Ziglio <frediano.ziglio@cloud.com>
>>
>> Currently .init section is both writeable and executable, split data and code
>> to have 2 sections satisfying W^X rule.
>>
>> It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
>> in the pagetables.
>>
>> NX_COMPAT is a requirement from shim-review,
>> https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility
>>
>> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
> Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>
> Is that the last piece necessary to satisfy the NX_COMPAT requirement? If
> so, I suppose a subsequent patch should actually set the
> IMAGE_DLLCHARACTERISTICS_NX_COMPAT bit (IIUC ld --nxcompat option), right?

The manpage says:

       --nxcompat
       --disable-nxcompat
           The image is compatible with the Data Execution Prevention. 
This feature was introduced with MS Windows XP SP2 for i386 PE targets. 
The option is enabled by default.

It turns out that Xen is being marked NX_COMPAT even prior to this
series, which is deeply suspicious as it has an RWX init section.

~Andrew

Re: [PATCH v3 4/4] x86: Split .init section to satisfy UEFI CA memory mitigation

[Jan Beulich]

On 16.06.2026 16:58, Andrew Cooper wrote:
> On 16/06/2026 12:20 pm, Marek Marczykowski-Górecki wrote:
>> On Tue, Jun 16, 2026 at 11:13:36AM +0100, Frediano Ziglio wrote:
>>> From: Frediano Ziglio <frediano.ziglio@cloud.com>
>>>
>>> Currently .init section is both writeable and executable, split data and code
>>> to have 2 sections satisfying W^X rule.
>>>
>>> It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
>>> in the pagetables.
>>>
>>> NX_COMPAT is a requirement from shim-review,
>>> https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility
>>>
>>> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
>> Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
>>
>> Is that the last piece necessary to satisfy the NX_COMPAT requirement? If
>> so, I suppose a subsequent patch should actually set the
>> IMAGE_DLLCHARACTERISTICS_NX_COMPAT bit (IIUC ld --nxcompat option), right?
> 
> The manpage says:
> 
>        --nxcompat
>        --disable-nxcompat
>            The image is compatible with the Data Execution Prevention. 
> This feature was introduced with MS Windows XP SP2 for i386 PE targets. 
> The option is enabled by default.
> 
> It turns out that Xen is being marked NX_COMPAT even prior to this
> series, which is deeply suspicious as it has an RWX init section.

The defaults used by ld are (imo wrongly) driven by Cygwin's / MinGW's
desires. They apparently assert that all binaries should be NX-compatible,
with merely a way to opt out. There's no dependency on section attributes
afaict.

Jan

Re: [PATCH v3 4/4] x86: Split .init section to satisfy UEFI CA memory mitigation

[Marek Marczykowski-Górecki]

[-- Attachment #1: Type: text/plain, Size: 1772 bytes --]

On Tue, Jun 16, 2026 at 03:58:27PM +0100, Andrew Cooper wrote:
> On 16/06/2026 12:20 pm, Marek Marczykowski-Górecki wrote:
> > On Tue, Jun 16, 2026 at 11:13:36AM +0100, Frediano Ziglio wrote:
> >> From: Frediano Ziglio <frediano.ziglio@cloud.com>
> >>
> >> Currently .init section is both writeable and executable, split data and code
> >> to have 2 sections satisfying W^X rule.
> >>
> >> It is a requirement for NX_COMPAT so the PE can be loaded with W^X perms
> >> in the pagetables.
> >>
> >> NX_COMPAT is a requirement from shim-review,
> >> https://github.com/rhboot/shim-review#do-you-have-the-nx-bit-set-in-your-shim-if-so-is-your-entire-boot-stack-nx-compatible-and-what-testing-have-you-done-to-ensure-such-compatibility
> >>
> >> Signed-off-by: Frediano Ziglio <frediano.ziglio@citrix.com>
> > Acked-by: Marek Marczykowski-Górecki <marmarek@invisiblethingslab.com>
> >
> > Is that the last piece necessary to satisfy the NX_COMPAT requirement? If
> > so, I suppose a subsequent patch should actually set the
> > IMAGE_DLLCHARACTERISTICS_NX_COMPAT bit (IIUC ld --nxcompat option), right?
> 
> The manpage says:
> 
>        --nxcompat
>        --disable-nxcompat
>            The image is compatible with the Data Execution Prevention. 
> This feature was introduced with MS Windows XP SP2 for i386 PE targets. 
> The option is enabled by default.
> 
> It turns out that Xen is being marked NX_COMPAT even prior to this
> series, which is deeply suspicious as it has an RWX init section.

My reading of binutils sources says it's enabled by default only for
mingw target. And indeed, inspection of xen.efi says only DYNAMIC_BASE
is set.

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]