linux-kernel - Re: [PATCH] arm64: fix non-converging vmlinux link

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] [day] [month] [year] [list]

Message-ID: <20210927104851.GA3292@C02TD0UTHF1T.local>
Date:   Mon, 27 Sep 2021 11:48:51 +0100
From:   Mark Rutland <mark.rutland@....com>
To:     Arnd Bergmann <arnd@...nel.org>
Cc:     Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>,
        Ard Biesheuvel <ardb@...nel.org>,
        Arnd Bergmann <arnd@...db.de>, Marc Zyngier <maz@...nel.org>,
        David Brazdil <dbrazdil@...gle.com>,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] arm64: fix non-converging vmlinux link

Hi Arnd, Ard,

On Mon, Sep 27, 2021 at 11:30:11AM +0200, Arnd Bergmann wrote:
> From: Ard Biesheuvel <ardb@...nel.org>
> 
> When the size of the vmlinux file is just below 64MB, the kernel
> may fail to link with lld, producing output such as
> 
> ld.lld: error: assignment to symbol init_pg_end does not converge
> ld.lld: error: assignment to symbol __pecoff_data_size does not converge
> 
> Change the INIT_DIR_SIZE definition to include init_pg_dir
                                        ^
Missing "not" here ---------------------'

We *need* to access `init_pg_dir` while using `init_pg_dir`, since e.g.
early_fixmap_init() needs to conntect the fixmap tables into it, so we
*must* map at least a portion of `init_pg_dir`.

We happen to over-map when using 4K pages, and so depending on the
alignment and size of the kernel Image this can work by chance. Also,
prior to v5.15-rc1, we'd over-map in all configurations, and this could
happen to work, but that was fixed in commit:

  90268574a3e8a6b8 ("arm64: head: avoid over-mapping in map_memory")

So as-is, this patch regresses working configurations at runtime,
including all 16K and 64K configs.

Is there some way we can over-estimate the size such that this will
converge? e.g. add some alignment padding such that `_end` won't
oscillate as `init_pg_end` changes?

Thanks,
Mark.

> to get a stable size calculation.
> 
> Arnd did the original report and analysis, but Ard figured what
> to do about and wrote the changes to the code.
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1219
> Co-developed-by: Ard Biesheuvel <ardb@...nel.org>
> Signed-off-by: Arnd Bergmann <arnd@...db.de>
> ---
> Ard, I had this in my randconfig tree with comment "Ard will
> submit this with a proper changelog", but it seems we both forgot
> about it, or maybe there was something wrong with it in the
> end.
> 
> While looking for randconfig -Werror warnings in mainline I came
> across it again and can confirm that this patch (or something like
> it) is still needed. Let me know if you are happy with this version
> or if you have a better description for it. I unfortunately forgot
> the details of how this works.
> ---
>  arch/arm64/include/asm/kernel-pgtable.h | 2 +-
>  arch/arm64/kernel/head.S                | 5 ++---
>  arch/arm64/kernel/vmlinux.lds.S         | 3 +++
>  3 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kernel-pgtable.h b/arch/arm64/include/asm/kernel-pgtable.h
> index 96dc0f7da258..5c622c18280a 100644
> --- a/arch/arm64/include/asm/kernel-pgtable.h
> +++ b/arch/arm64/include/asm/kernel-pgtable.h
> @@ -86,7 +86,7 @@
>  			+ EARLY_PGDS((vstart), (vend)) 	/* each PGDIR needs a next level page table */	\
>  			+ EARLY_PUDS((vstart), (vend))	/* each PUD needs a next level page table */	\
>  			+ EARLY_PMDS((vstart), (vend)))	/* each PMD needs a next level page table */
> -#define INIT_DIR_SIZE (PAGE_SIZE * EARLY_PAGES(KIMAGE_VADDR, _end))
> +#define INIT_DIR_SIZE (PAGE_SIZE * EARLY_PAGES(KIMAGE_VADDR, init_pg_dir))
>  #define IDMAP_DIR_SIZE		(IDMAP_PGTABLE_LEVELS * PAGE_SIZE)
>  
>  /* Initial memory map size */
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 17962452e31d..2c3011660e48 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -366,10 +366,9 @@ SYM_FUNC_START_LOCAL(__create_page_tables)
>  	mov_q	x5, KIMAGE_VADDR		// compile time __va(_text)
>  	add	x5, x5, x23			// add KASLR displacement
>  	mov	x4, PTRS_PER_PGD
> -	adrp	x6, _end			// runtime __pa(_end)
>  	adrp	x3, _text			// runtime __pa(_text)
> -	sub	x6, x6, x3			// _end - _text
> -	add	x6, x6, x5			// runtime __va(_end)
> +	sub	x6, x0, x3			// init_pg_dir - _text
> +	add	x6, x6, x5			// runtime __va(init_pg_dir)
>  
>  	map_memory x0, x1, x5, x6, x7, x3, x4, x10, x11, x12, x13, x14
>  
> diff --git a/arch/arm64/kernel/vmlinux.lds.S b/arch/arm64/kernel/vmlinux.lds.S
> index f6b1a88245db..4792ddd1ae73 100644
> --- a/arch/arm64/kernel/vmlinux.lds.S
> +++ b/arch/arm64/kernel/vmlinux.lds.S
> @@ -287,6 +287,9 @@ SECTIONS
>  	BSS_SECTION(SBSS_ALIGN, 0, 0)
>  
>  	. = ALIGN(PAGE_SIZE);
> +
> +	/* ----- kernel virtual mapping ends here ---- */
> +
>  	init_pg_dir = .;
>  	. += INIT_DIR_SIZE;
>  	init_pg_end = .;
> -- 
> 2.29.2
>