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

From: Russell King - ARM Linux admin <linux@armlinux.org.uk>
To: Geert Uytterhoeven <geert+renesas@glider.be>
Cc: "Arnd Bergmann" <arnd@arndb.de>,
	"Nicolas Pitre" <nico@fluxnic.net>,
	"Stephen Boyd" <sboyd@kernel.org>,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Lukasz Stelmach" <l.stelmach@samsung.com>,
	linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org,
	"Chris Brandt" <chris.brandt@renesas.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Eric Miao" <eric.miao@nvidia.com>,
	"Dmitry Osipenko" <digetx@gmail.com>,
	"Ard Biesheuvel" <ardb@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	"Marek Szyprowski" <m.szyprowski@samsung.com>
Subject: Re: [PATCH v10 3/3] ARM: uncompress: Validate start of physical memory against passed DTB
Date: Thu, 3 Dec 2020 14:04:50 +0000	[thread overview]
Message-ID: <20201203140450.GH1551@shell.armlinux.org.uk> (raw)
In-Reply-To: <20201203121916.2870975-4-geert+renesas@glider.be>

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!

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