Re: [PATCH v2 4/5] x86/xen: Avoid relocatable quantities in Xen ELF notes

[Jason Andryuk]

On 2024-09-30 03:15, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@xxxxxxxxxx>
>
> Xen puts virtual and physical addresses into ELF notes that are treated
> by the linker as relocatable by default. Doing so is not only pointless,
> given that the ELF notes are only intended for consumption by Xen before
> the kernel boots. It is also a KASLR leak, given that the kernel's ELF
> notes are exposed via the world readable /sys/kernel/notes.
>
> So emit these constants in a way that prevents the linker from marking
> them as relocatable. This involves place-relative relocations (which
> subtract their own virtual address from the symbol value) and linker
> provided absolute symbols that add the address of the place to the
> desired value.
>
> Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>

Tested-by: Jason Andryuk <jason.andryuk@xxxxxxx>

The generated values look ok.

> ---
>   arch/x86/kernel/vmlinux.lds.S | 13 +++++++++++++
>   arch/x86/platform/pvh/head.S  |  6 +++---
>   arch/x86/tools/relocs.c       |  1 +
>   arch/x86/xen/xen-head.S       |  6 ++++--
>   4 files changed, 21 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> index 6726be89b7a6..2b7c8c14c6fd 100644
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -527,3 +527,16 @@ INIT_PER_CPU(irq_stack_backing_store);
>   #endif
>   
>   #endif /* CONFIG_X86_64 */
> +
> +#ifdef CONFIG_XEN
> +#ifdef CONFIG_XEN_PV
> +xen_elfnote_entry_offset =
> +	ABSOLUTE(xen_elfnote_entry) + ABSOLUTE(startup_xen);
> +#endif
> +xen_elfnote_hypercall_page_offset =
> +	ABSOLUTE(xen_elfnote_hypercall_page) + ABSOLUTE(hypercall_page);
> +#endif
> +#ifdef CONFIG_PVH
> +xen_elfnote_phys32_entry_offset =
> +	ABSOLUTE(xen_elfnote_phys32_entry) + ABSOLUTE(pvh_start_xen - LOAD_OFFSET);
> +#endif

It seems to me, these aren't really offsets, but instead an address + value.

> diff --git a/arch/x86/platform/pvh/head.S b/arch/x86/platform/pvh/head.S
> index 7ca51a4da217..2b0d887e0872 100644
> --- a/arch/x86/platform/pvh/head.S
> +++ b/arch/x86/platform/pvh/head.S

> @@ -300,5 +300,5 @@ SYM_DATA_END(pvh_level2_kernel_pgt)
>   		     .long KERNEL_IMAGE_SIZE - 1)
>   #endif
>   
> -	ELFNOTE(Xen, XEN_ELFNOTE_PHYS32_ENTRY,
> -	             _ASM_PTR (pvh_start_xen - __START_KERNEL_map))
> +	ELFNOTE(Xen, XEN_ELFNOTE_PHYS32_ENTRY, .global xen_elfnote_phys32_entry;
> +		xen_elfnote_phys32_entry: _ASM_PTR xen_elfnote_phys32_entry_offset - .)

So here you have `address + value - address` to put the desired value in 
the elf note?

Regards,
Jason