linux-kernel - Re: [RFC PATCH] riscv: vdso: remove hardcoded 0x800 .text section start addr

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: <20221124103255.33prhf3gf6xtwlsk@kamzik>
Date:   Thu, 24 Nov 2022 11:32:55 +0100
From:   Andrew Jones <ajones@...tanamicro.com>
To:     Jisheng Zhang <jszhang@...nel.org>
Cc:     Paul Walmsley <paul.walmsley@...ive.com>,
        Palmer Dabbelt <palmer@...belt.com>,
        Albert Ou <aou@...s.berkeley.edu>,
        linux-riscv@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH] riscv: vdso: remove hardcoded 0x800 .text section
 start addr

On Thu, Nov 24, 2022 at 12:18:05AM +0800, Jisheng Zhang wrote:
> It seems the hardcoded 0x800 isn't necessary, but removing it brings a

s/, but/and/

> small vdso.so and aligns with other architectures.

This commit message didn't really satisfy my desire to understand why
the comment and '. = 0x800' were there in the first place and if its safe
to remove now, so I tried to do some of my own digging. I found

commit 5b9304933730 ("x86 vDSO: generate vdso-syms.lds")
commit f6b46ebf904f ("x86 vDSO: new layout")

which removes the comment and hard coding for x86 by changing the vdso
Makefile. Then looking at

commit 9031fefde6f2 ("arm64: VDSO support")

we see that it starts with the new Makefile approach and doesn't bother
with the hard coding from the start. As riscv also started with the new
Makefile approach it also could have dropped the hard coding from the
start (I guess).

> 
> Signed-off-by: Jisheng Zhang <jszhang@...nel.org>
> ---
>  arch/riscv/kernel/vdso/vdso.lds.S | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S
> index 150b1a572e61..7be7e618d59c 100644
> --- a/arch/riscv/kernel/vdso/vdso.lds.S
> +++ b/arch/riscv/kernel/vdso/vdso.lds.S
> @@ -31,13 +31,7 @@ SECTIONS
>  
>  	.rodata		: { *(.rodata .rodata.* .gnu.linkonce.r.*) }
>  
> -	/*
> -	 * This linker script is used both with -r and with -shared.
> -	 * For the layouts to match, we need to skip more than enough
> -	 * space for the dynamic symbol table, etc. If this amount is
> -	 * insufficient, ld -shared will error; simply increase it here.
> -	 */
> -	. = 0x800;
> +	. = ALIGN(4);

I realize 4 is used here now because I questioned the 16, but after doing
my digging I think a larger alignment may be better. Loading the text may
be done with 8 byte or larger reads, so having the section aligned to a
larger size would be better reading it. We might as well use 16, like
arm64 does, and like you had before?

Also, having enough separation between data and text seems to be
important for cache reasons, based on the comment in
./arch/x86/entry/vdso/vdso-layout.lds.S and other vdso history.
Maybe we should move .note, .eh_frame_hdr, and .eh_frame below
.rodata like x86 has it?

Thanks,
drew

>  	.text		: { *(.text .text.*) }		:text
>  
>  	.data		: {
> -- 
> 2.37.2
>