Re: [PATCH v5 1/4] arm64: vdso: put ELF related sections in the linker script

From: Vincenzo Frascino <vincenzo.frascino@arm.com>
To: Joey Gouly <joey.gouly@arm.com>, linux-arm-kernel@lists.infradead.org
Cc: nd@arm.com, catalin.marinas@arm.com, keescook@chromium.org,
	masahiroy@kernel.org, will@kernel.org
Subject: Re: [PATCH v5 1/4] arm64: vdso: put ELF related sections in the linker script
Date: Tue, 10 May 2022 11:39:19 +0100	[thread overview]
Message-ID: <00abb0c5-6360-0004-353f-e7a88b3bd22c@arm.com> (raw)
In-Reply-To: <20220510095834.32394-2-joey.gouly@arm.com>

Hi Joey,

On 5/10/22 10:58, Joey Gouly wrote:
> Use macros from vmlinux.lds.h to explicitly name sections that are included
> in the VDSO output.
> 
> Signed-off-by: Joey Gouly <joey.gouly@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: Will Deacon <will@kernel.org>
> Cc: Masahiro Yamada <masahiroy@kernel.org>
> Cc: Vincenzo Frascino <vincenzo.frascino@arm.com>
> Cc: Kees Cook <keescook@chromium.org>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: Vincenzo Frascino <vincenzo.frascino@arm.com>

As a general rule, when you make a consistent change to a patch that was
previously reviewed you should drop the "Reviewed-by:" tag as an indication that
the patch needs to be reviewed again.

> ---
>  arch/arm64/kernel/vdso/vdso.lds.S | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/arch/arm64/kernel/vdso/vdso.lds.S b/arch/arm64/kernel/vdso/vdso.lds.S
> index a5e61e09ea92..1cd85258c079 100644
> --- a/arch/arm64/kernel/vdso/vdso.lds.S
> +++ b/arch/arm64/kernel/vdso/vdso.lds.S
> @@ -11,6 +11,7 @@
>  #include <linux/const.h>
>  #include <asm/page.h>
>  #include <asm/vdso.h>
> +#include <asm-generic/vmlinux.lds.h>
>  
>  OUTPUT_FORMAT("elf64-littleaarch64", "elf64-bigaarch64", "elf64-littleaarch64")
>  OUTPUT_ARCH(aarch64)
> @@ -49,11 +50,26 @@ SECTIONS
>  
>  	.dynamic	: { *(.dynamic) }		:text	:dynamic
>  
> +	.rela.dyn	: ALIGN(8) { *(.rela .rela*) }
> +
> +	.plt		: {
> +		*(.plt)
> +		*(.plt.*)
> +		*(.iplt)
> +		*(.igot .igot.plt)
> +	}
> +
> +	.got		: { *(.got) }
> +	.got.plt	: { *(.got.plt) }
> +

The vDSO library does not contain absolute relocations. Is there any specific
reason why these sections are not part of ".rodata"?

Please refer to: lib/vdso/Makefile

Note: A similar comment applies to vdso32.

>  	.rodata		: { *(.rodata*) }		:text
>  
>  	_end = .;
>  	PROVIDE(end = .);
>  
> +	DWARF_DEBUG
> +	ELF_DETAILS
> +
>  	/DISCARD/	: {
>  		*(.data .data.* .gnu.linkonce.d.* .sdata*)
>  		*(.bss .sbss .dynbss .dynsbss)

-- 
Regards,
Vincenzo

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel