linux-kernel - Re: [PATCH v10 3/3] ARM: uncompress: Validate start of physical memory against passed DTB

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20201203140450.GH1551@shell.armlinux.org.uk>
Date:   Thu, 3 Dec 2020 14:04:50 +0000
From:   Russell King - ARM Linux admin <linux@...linux.org.uk>
To:     Geert Uytterhoeven <geert+renesas@...der.be>
Cc:     Nicolas Pitre <nico@...xnic.net>, Ard Biesheuvel <ardb@...nel.org>,
        Marek Szyprowski <m.szyprowski@...sung.com>,
        Dmitry Osipenko <digetx@...il.com>,
        Linus Walleij <linus.walleij@...aro.org>,
        Arnd Bergmann <arnd@...db.de>,
        Eric Miao <eric.miao@...dia.com>,
        Uwe Kleine-König 
        <u.kleine-koenig@...gutronix.de>,
        Lukasz Stelmach <l.stelmach@...sung.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Chris Brandt <chris.brandt@...esas.com>,
        linux-arm-kernel@...ts.infradead.org,
        linux-renesas-soc@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v10 3/3] ARM: uncompress: Validate start of physical
 memory against passed DTB

On Thu, Dec 03, 2020 at 01:19:16PM +0100, Geert Uytterhoeven wrote:
> diff --git a/arch/arm/boot/compressed/head.S b/arch/arm/boot/compressed/head.S
> index d9cce7238a365081..1b6425df87e84e71 100644
> --- a/arch/arm/boot/compressed/head.S
> +++ b/arch/arm/boot/compressed/head.S
> @@ -282,10 +282,36 @@ not_angel:
>  		 * are already placing their zImage in (eg) the top 64MB
>  		 * of this range.
>  		 */
> -		mov	r4, pc
> -		and	r4, r4, #0xf8000000
> +		mov	r0, pc
> +		and	r0, r0, #0xf8000000
> +#ifdef CONFIG_USE_OF
> +		adr	r1, LC1
> +#ifdef CONFIG_ARM_APPENDED_DTB
> +		/*
> +		 * Look for an appended DTB.  If found, we cannot use it to
> +		 * validate the calculated start of physical memory, as its
> +		 * memory nodes may need to be augmented by ATAGS stored at
> +		 * an offset from the same start of physical memory.
> +		 */
> +		ldr	r2, [r1, #4]	@ get &_edata
> +		add	r2, r2, r1	@ relocate it
> +		ldr	r2, [r2]	@ get DTB signature
> +		ldr	r3, =OF_DT_MAGIC
> +		cmp	r2, r3		@ do we have a DTB there?
> +		beq	1f		@ if yes, skip validation
> +#endif /* CONFIG_ARM_APPENDED_DTB */
> +
> +		/* Make sure we have some stack */
> +		ldr	sp, [r1]	@ get stack location
> +		add	sp, sp, r1	@ apply relocation
> +
> +		/* Validate calculated start against passed DTB */
> +		mov	r1, r8
> +		bl	fdt_check_mem_start

I don't think this is going to work. You can only run C code when
it has been linked for a specific address and is loaded at the
correct address as it contains absolute addresses. We work around
that in the decompressor by (ab)using the GOT table, and the code
that fixes up the GOT table is run later, after you've called out
to some C code here.

Quite how this works for you without the GOT fixups having been
done, I'm not sure.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!