Message ID | 20250401130840.72119-9-roger.pau@citrix.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | x86/EFI: prevent write-execute sections | expand |
On 01/04/2025 2:08 pm, Roger Pau Monne wrote: > The trampoline code is never executed in the position placed by the > loader. It's first copied to the low 1MB, and always executed from > there. > > Move the trampoline code from being in .init.text section into > .init.data, so it's not in an executable section. This allows applying > the relocations safely against a non-executable (and thus non-read only) > section, and then copy the relocated trampoline to the low 1MB. Note > that the trampoline code is placed on an .init section, so zapped after > boot has finished. > > No functional change intended. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > An alternative approach is to apply the relocations after having copied the > trampoline to the low 1MB, but that still generates relocations in mkreloc, > which is not helpful for the goal of having no relocations applied to > read-execute code sections. This approach however places code in a data > section, which might cause issues when debugging it. I, probably most of all, spend a reasonable amount of time disassembling the trampoline. I really would prefer to keep it in an executable section. What are the options with relocations? Can't we simply drop any in the trampoline region? ~Andrew
On 01.04.2025 15:49, Andrew Cooper wrote: > On 01/04/2025 2:08 pm, Roger Pau Monne wrote: >> The trampoline code is never executed in the position placed by the >> loader. It's first copied to the low 1MB, and always executed from >> there. >> >> Move the trampoline code from being in .init.text section into >> .init.data, so it's not in an executable section. This allows applying >> the relocations safely against a non-executable (and thus non-read only) >> section, and then copy the relocated trampoline to the low 1MB. Note >> that the trampoline code is placed on an .init section, so zapped after >> boot has finished. >> >> No functional change intended. >> >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >> --- >> An alternative approach is to apply the relocations after having copied the >> trampoline to the low 1MB, but that still generates relocations in mkreloc, >> which is not helpful for the goal of having no relocations applied to >> read-execute code sections. This approach however places code in a data >> section, which might cause issues when debugging it. > > I, probably most of all, spend a reasonable amount of time disassembling > the trampoline. I really would prefer to keep it in an executable section. > > What are the options with relocations? Can't we simply drop any in the > trampoline region? That's perhaps an option with mkreloc, but would require zapping them after linking if we have the linker produce relocations. Doable, but likely not to end up being pretty. Jan
diff --git a/xen/arch/x86/boot/head.S b/xen/arch/x86/boot/head.S index 774894954e44..f5a2d08d0d0e 100644 --- a/xen/arch/x86/boot/head.S +++ b/xen/arch/x86/boot/head.S @@ -709,7 +709,7 @@ trampoline_setup: /* Jump into the relocated trampoline. */ lret - .section .init.text, "ax", @progbits + .section .init.data, "aw", @progbits ENTRY(trampoline_start) #include "trampoline.S" ENTRY(trampoline_end)
The trampoline code is never executed in the position placed by the loader. It's first copied to the low 1MB, and always executed from there. Move the trampoline code from being in .init.text section into .init.data, so it's not in an executable section. This allows applying the relocations safely against a non-executable (and thus non-read only) section, and then copy the relocated trampoline to the low 1MB. Note that the trampoline code is placed on an .init section, so zapped after boot has finished. No functional change intended. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- An alternative approach is to apply the relocations after having copied the trampoline to the low 1MB, but that still generates relocations in mkreloc, which is not helpful for the goal of having no relocations applied to read-execute code sections. This approach however places code in a data section, which might cause issues when debugging it. --- xen/arch/x86/boot/head.S | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)