| Message ID | 20250401130840.72119-10-roger.pau@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/EFI: prevent write-execute sections | expand |
On 01.04.2025 15:08, Roger Pau Monne wrote: > 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 loader has acknowledged the Xen image section > attributes, and mapped .init.text with just read and execute permissions. Fine in principle, but ... > --- a/xen/arch/x86/efi/efi-boot.h > +++ b/xen/arch/x86/efi/efi-boot.h > @@ -266,7 +266,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" > @@ -274,7 +276,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(); > } ... imo ought to come with a brief comment, to keep people from trying to undo ("simplify") that again. [offset]'s constraint could in principle be "rme", I think, as [rip] is "&r" already. Just that the compiler (at least gcc) won't synthesize a memory operand, and the value can't be expressed by an immediate. IOW - probably all fine with just "r". Of course if/when we add further operands here, we need to pay attention to the number of registers needed. Jan
diff --git a/xen/arch/x86/efi/efi-boot.h b/xen/arch/x86/efi/efi-boot.h index 1d8902a9a724..c5cbf56cc0c4 100644 --- a/xen/arch/x86/efi/efi-boot.h +++ b/xen/arch/x86/efi/efi-boot.h @@ -266,7 +266,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" @@ -274,7 +276,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(); }
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 loader has acknowledged the Xen image section attributes, and mapped .init.text with just read and execute permissions. No functional change intended. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/efi/efi-boot.h | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)