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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b2b23136-fd57-4bc8-bb80-604cc4da0920@amd.com>
Date: Wed, 2 Oct 2024 17:25:19 -0400
From: Jason Andryuk <jason.andryuk@....com>
To: Ard Biesheuvel <ardb+git@...gle.com>, <linux-kernel@...r.kernel.org>
CC: Ard Biesheuvel <ardb@...nel.org>, Juergen Gross <jgross@...e.com>, "Boris
 Ostrovsky" <boris.ostrovsky@...cle.com>, <x86@...nel.org>,
	<xen-devel@...ts.xenproject.org>
Subject: Re: [PATCH v2 4/5] x86/xen: Avoid relocatable quantities in Xen ELF
 notes

On 2024-09-30 03:15, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@...nel.org>
> 
> 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@...nel.org>

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

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