linux-kernel - Re: [PATCH] powerpc: Fix kernel-jump address for ppc64 wrapper boot

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: <c80f69d0-44b0-24a6-ce4f-ed5a40514597@csgroup.eu>
Date:   Tue, 8 Jun 2021 06:55:35 +0200
From:   Christophe Leroy <christophe.leroy@...roup.eu>
To:     He Ying <heying24@...wei.com>, mpe@...erman.id.au,
        benh@...nel.crashing.org, paulus@...ba.org, nathan@...nel.org
Cc:     linuxppc-dev@...ts.ozlabs.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] powerpc: Fix kernel-jump address for ppc64 wrapper boot



Le 04/06/2021 à 11:22, He Ying a écrit :
>  From "64-bit PowerPC ELF Application Binary Interface Supplement 1.9",
> we know that the value of a function pointer in a language like C is
> the address of the function descriptor and the first doubleword
> of the function descriptor contains the address of the entry point
> of the function.
> 
> So, when we want to jump to an address (e.g. addr) to execute for
> PPC-elf64abi, we should assign the address of addr *NOT* addr itself
> to the function pointer or system will jump to the wrong address.
> 
> Link: https://refspecs.linuxfoundation.org/ELF/ppc64/PPC-elf64abi.html#FUNC-DES
> Signed-off-by: He Ying <heying24@...wei.com>
> ---
>   arch/powerpc/boot/main.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
> 
> diff --git a/arch/powerpc/boot/main.c b/arch/powerpc/boot/main.c
> index cae31a6e8f02..50fd7f11b642 100644
> --- a/arch/powerpc/boot/main.c
> +++ b/arch/powerpc/boot/main.c
> @@ -268,7 +268,16 @@ void start(void)
>   	if (console_ops.close)
>   		console_ops.close();
>   
> +#ifdef CONFIG_PPC64_BOOT_WRAPPER

This kind of need doesn't desserve a #ifdef, see 
https://www.kernel.org/doc/html/latest/process/coding-style.html#conditional-compilation

You can do:


	kentry = (kernel_entry_t)(IS_ENABLED(CONFIG_PPC64_BOOT_WRAPPER) ? &vmlinux.addr : vmlinux.addr);


Or, if you prefer something less compact:


	if (IS_ENABLED(CONFIG_PPC64_BOOT_WRAPPER))
		kentry = (kernel_entry_t) &vmlinux.addr;
	else
		kentry = (kernel_entry_t) vmlinux.addr;


> +	/*
> +	 * For PPC-elf64abi, the value of a function pointer is the address
> +	 * of the function descriptor. And the first doubleword of a function
> +	 * descriptor contains the address of the entry point of the function.
> +	 */
> +	kentry = (kernel_entry_t) &vmlinux.addr;
> +#else
>   	kentry = (kernel_entry_t) vmlinux.addr;
> +#endif
>   	if (ft_addr) {
>   		if(platform_ops.kentry)
>   			platform_ops.kentry(ft_addr, vmlinux.addr);
>