Re: [PATCH 1/2] ARM: i.MX: tqma6ulx: fix barebox chainloading with OP-TEE enabled

[Sascha Hauer <s.hauer@pengutronix.de>]

On Thu, Jun 26, 2025 at 05:37:35PM +0200, Ahmad Fatoum wrote:
> Hello Sascha,
> 
> On 6/26/25 16:03, Sascha Hauer wrote:
> > Chainloading barebox when OP-TEE is enabled has multiple bugs, fix them.
> > 
> > When barebox starts we have to guess if we have to start OP-TEE or not.
> > As we can't detect the exception level on ARMv7 we do this by checking
> > if a first stage loader has passed us a device tree.
> > 
> > First of all the device tree is passed in r2, not in r0, so fix the
> > register we use.
> > 
> > Then we have to check if r2 is within the SDRAM. We check against
> > MX6_MMDC_P0_BASE_ADDR which is the base of the SDRAM controller. Use the
> > base address of the SDRAM instead.
> > 
> > Finally we manipulate the TZASC which we are obviously not allowed in
> > EL1, so move the manipulation to the code which is only executed in EL2.
> 
> EL1/EL2 are ARMv8-specific.

What's the correct term for ARMv7 then? Supervisor mode? My google foo
always points me to the ARMv8 terms.

> >  	if (IS_ENABLED(CONFIG_FIRMWARE_TQMA6UL_OPTEE) &&
> > -	    !(r0 > MX6_MMDC_P0_BASE_ADDR &&
> > -	      r0 < MX6_MMDC_P0_BASE_ADDR + SZ_256M)) {
> > +	    !(r2 > MX6_MMDC_PORT0_BASE_ADDR && r2 < MX6_MMDC_PORT0_BASE_ADDR + SZ_256M)) {
> > +		/* Enable normal/secure r/w for TZC380 region0 */
> > +		writel(0xf0000000, 0x021D0108);
> 
> I think this is problematic:
> 
>   - robustness-wise: We have no guarantee that there isn't some lesser
>     used BootROM code path that happens to leave a suitable DRAM
>     look-alike address that would trick us here.
> 
>   - security wise, even if we check for FDT header if r2 points into
>     DRAM, a compromised OS could spray RAM with FDT magic,
>     trigger a warm reset that has the BootROM produce a DRAM lookalike
>     pointer in r2 and then OP-TEE loading is skipped and the kernel
>     starts in the highest privilege level.
> 
> To address this, we need some way to set a sticky bit that's cleared
> only on reset. One way, would be to set up an IVT and try to access the
> L2 cache controller while data_abort_mask() is active, like
> imx6_cannot_write_l2x0 is doing.

I'll send an updated series shortly.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |