[PATCH RFC] lib: optee: forbid OP-TEE OS loading without adding OP-TEE OS reserved-memory nodes

[Tom Rini]

On Mon, Sep 22, 2025 at 11:29:07AM +0200, Quentin Schulz wrote:
> Hi Tom,
> 
> On 9/19/25 6:10 PM, Tom Rini wrote:
> > On Fri, Sep 19, 2025 at 05:50:21PM +0200, Quentin Schulz wrote:
> > 
> > > From: Quentin Schulz <quentin.schulz at cherry.de>
> > > 
> > > It is currently possible to build an image with an OP-TEE OS (via the
> > > TEE environment variable) without OPTEE_LIB. U-Boot will happily load
> > > the TEE OS and the next OS (e.g. the Linux kernel).
> > > 
> > > This is an issue because on FDT-enabled devices, OP-TEE OS adds nodes to
> > > the reserved-memory FDT node for the memory regions it just reserved for
> > > itself. This updated is then passed to U-Boot proper which should know
> > > better not to use memory from there. The actual issue is that without
> > > OPTEE_LIB and OF_LIBFDT enabled, U-Boot proper will not copy those nodes
> > > over to the next OS's FDT before starting it. This results in the next
> > > OS's (e.g. Linux kernel) to not be aware of reserved memory, incurring
> > > random crashes or device reboots when it tries to access secure reserved
> > > memory area.
> > > 
> > > Signed-off-by: Quentin Schulz <quentin.schulz at cherry.de>
> > > ---
> > > I've just lost a day trying to figure out why my board suddenly boot
> > > loops when running a specific program in Linux userspace after updating
> > > U-Boot. I actually inadvertently had the TEE environment variable set
> > > for a device which doesn't actually need to run any TEE OS (so had
> > > OPTEE_LIB disabled).
> > > 
> > > This device is Rockchip PX30-based and the image is built with binman
> > > like all Rockchip-based devices.
> > > 
> > > The issue is that I built a U-Boot FIT with an OP-TEE OS that reserves
> > > some memory regions for itself (via the TEE environment variable) but
> > > U-Boot proper "forgot" to propagate those to the Linux kernel's FDT
> > > (because of the missing OPTEE_LIB).
> > > 
> > > Unfortunately, binman doesn't seem to have access to Kconfig symbols
> > > (grep CONFIG_ doesn't return anything meaningful and binman is either
> > >   configured through FDT nodes or via CLI arguments, c.f. cmd_binman in
> > >   the root Makefile) so I cannot try to be smart and guide the user to
> > > the correct Kconfig option to select if TEE is set. I could add a
> > > property based on the presence of OPTEE_LIB in rockchip-u-boot.dtsi for
> > > example and have a custom message based on that, the issue is that I
> > > assume all FDT-based platforms do actually need to do this dance, and
> > > not only Rockchip.
> > > Another option could be to add a CLI argument to binman through which
> > > we would pass the state of OPTEE_LIB and error out the build in that
> > > case, but that feels like opening the door to dirty hacks (though this
> > > patch is admittedly another dirty hack :) ).
> > > 
> > > Another option is to propagate the TEE environment variable to the
> > > preprocessor of the FDT (via dtc_cpp_flags) and then I can do
> > > 
> > >   #if defined(TEE) && !IS_ENABLED(CONFIG_OPTEE_LIB)
> > >   #error "CONFIG_OPTEE_LIB must be enabled!"
> > >   #endif
> > > 
> > > but we have the same issue as above, it is then Rockchip specific and
> > > also feels bad.
> > > 
> > > Another option is to remove the @tee-SEQ node from the binman FIT
> > > description when OPTEE_LIB isn't set but then I would lose the nice
> > > message when no TEE is provided:
> > > 
> > > Image 'simple-bin' is missing optional external blobs but is still functional: tee-os
> > > 
> > > and even worse, build without any TEE OS even though I could provide one
> > > with the TEE environment variable.
> > > 
> > > Finally, another option could be to move this hack under
> > > arch/arm/mach-rockchip/Kconfig to make it Rockchip specific or add a
> > > depends on ARCH_ROCKCHIP
> > > 
> > > OP-TEE OS on Aarch32 Rockchip boards doesn't actually need any of that
> > > if SPL_OPTEE_IMAGE is set because arch/arm/mach-rockchip/sdram.c then
> > > marks some hardcoded memory regions in RAM as holes in DRAM, which has
> > > the same effect as reserved memory regions I guess. I assume other
> > > platforms may use something different, so it may be casting too wide of
> > > a net.
> > > 
> > > Does anyone have something more delicate than this sledgehammer of a
> > > solution?
> > 
> > It looks like on other SoCs there's an ARCH_x wide imply/select OPTEE
> > (which in turn gets OPTEE_LIB), so is there a case for ARCH_ROCKCHIP &&
> > !ARM64 && !OPTEE ? I see you explained that 32bit has a workaround for
> > this already.
> > 
> 
> The issue is that on Rockchip you don't need OPTEE to be selected for the
> OP-TEE OS to be integrated in the U-Boot FIT image. You simply set the TEE
> environment variable to the path to the bl32 binary and it's going to
> include it in the U-Boot FIT.
> 
> I think this is actually a good thing to be allowed to have the OP-TEE OS
> image **without** the OPTEE driver being compiled: if I want OP-TEE OS to be
> loaded in the system, this will be done by the SPL by loading the binary at
> a specific location in RAM, but I may not want to increase the attack
> surface by enabling the OP-TEE driver in U-Boot if I don't need to interact
> with OP-TEE OS from U-Boot at all.
> 
> The Aarch64 Rockchip SoCs also do not require OP-TEE OS to work (but I
> **think** Aarch32 SoCs do?). "imply" in that case would not be enough
> because we would still be able to include the OP-TEE OS image in the U-Boot
> FIT without OPTEE driver compiled in (and thus OPTEE_LIB). "select" would be
> too strong, because we really don't need OP-TEE OS (or rather its driver,
> but the latter kind of implies the former) to be in the image in order for
> the kernel to work just fine.
> 
> It all boils down to the ability to add OP-TEE OS to U-Boot FIT via the TEE
> environment variable without changing the defconfig (i.e. it's optional). We
> could make the @tee-SEQ node in arch/arm/dts/rockchip-u-boot.dtsi presence
> depend on OPTEE_LIB for example, but then we wouldn't be able to tell the
> user that the TEE OS image is missing. Also, this would limit TEE OS to
> OP-TEE OS (which I assume is probably what everybody is using on Rockchip
> but we could be using something else technically?). That may be an
> acceptable limitation though!
> 
> As for forcing OPTEE_LIB on Aarch32 Rockchip SoCs on, this OPTEE_LIB thing
> relies on OP-TEE OS patching the DTB passed to U-Boot proper so that the
> reserved memory nodes for OP-TEE are made explicit. I somehow doubt this is
> necessarily handled by OP-TEE blobs from Rockchip? Which means that I'm not
> sure that switching the hardcoded implementation to use the logic in
> OPTEE_LIB will be enough.
> This also reminds me that TF-A blobs from Rockchip (used to?) crash for some
> DTBs, which was worked around by simply just no passing the DTB to TF-A (see
> SPL_ATF_NO_PLATFORM_PARAM), which I assume means OP-TEE OS doesn't have
> access to a DTB so cannot patch it. I'm not sure how (if!) it is handled by
> U-Boot in that case.

Getting back to this, thanks for detailing the explanation. I could not
think of a more clever way to deal with the problem you're describing,
so whatever you think looks sufficiently clean, please non-RFC and this
is a case where a long commit message is likely good for future
reference in case someone wants to clean this up. Thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20251017/e7a7158a/attachment.sig>