From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michal Simek Date: Thu, 10 Sep 2020 15:38:25 +0200 Subject: [PATCH v3 4/4] arm64: Trap non-PIE builds early if starting from wrong address In-Reply-To: References: <20200909170727.658573-1-edgar.iglesias@gmail.com> <20200909170727.658573-5-edgar.iglesias@gmail.com> <27b8fa20-fc4a-0a56-940b-40d3e740d9f0@xilinx.com> Message-ID: <6221239e-e459-859a-5770-ea987e164640@xilinx.com> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 10. 09. 20 15:06, Andr? Przywara wrote: > On 10/09/2020 13:38, Michal Simek wrote: >> >> >> On 09. 09. 20 19:07, Edgar E. Iglesias wrote: >>> From: "Edgar E. Iglesias" >>> >>> Trap non-PIE builds early if the start address doesn't match >>> between run-time and link-time. This will trap the startup >>> sequence rather than letting it run into obscure errors. >>> >>> Signed-off-by: Edgar E. Iglesias >>> --- >>> arch/arm/cpu/armv8/start.S | 13 +++++++++++++ >>> 1 file changed, 13 insertions(+) >>> >>> diff --git a/arch/arm/cpu/armv8/start.S b/arch/arm/cpu/armv8/start.S >>> index e5c2856cf5..39e1b842c4 100644 >>> --- a/arch/arm/cpu/armv8/start.S >>> +++ b/arch/arm/cpu/armv8/start.S >>> @@ -101,6 +101,19 @@ pie_skip_reloc: >>> cmp x2, x3 >>> b.lo pie_fix_loop >>> pie_fixup_done: >>> +#else >>> + adr x0, _start >>> + ldr x1, _TEXT_BASE >>> + cmp x0, x1 >>> + beq 1f >>> +0: >>> + /* >>> + * FATAL, can't continue. >>> + * U-Boot needs to start executing at CONFIG_SYS_TEXT_BASE. >>> + */ >>> + wfi >>> + b 0b >>> +1: >>> #endif >>> >>> #ifdef CONFIG_SYS_RESET_SCTRL >>> >> >> NACK for this. >> >> 1. It breaks SPL flow because CONFIG_SYS_TEXT_BASE is text base for >> U-Boot proper >> 2. It likely also breaks TPL flow for the same reason >> >> 3. And last thing is that this code is used only for U-Boot proper. >> .globl _TEXT_BASE >> _TEXT_BASE: >> .quad CONFIG_SYS_TEXT_BASE >> >> The fixes are below. Point 3 should be likely be in separate patch >> because it is unrelated. > > So if this patch causes issues, can't we just drop it? I mean right now > you will probably just crash anyway if you load it at the wrong address, > but maybe late enough that you get more hints or even some output. > > Now this patch makes sure that you don't get anything, so I don't see > how this is really improving the situation. It seems like a case of > "don't fix things that ain't broken". I am fine with dropping it. Tom: What do you think? Thanks, Michal