Re: [PATCH v6 1/9] spl: Kconfig: allow K3 devices to use falcon mode

[Anshul Dalal <anshuld@ti.com>]

On Thu May 29, 2025 at 7:06 AM IST, Andrew Davis wrote:
> On 5/28/25 8:08 PM, Anshul Dalal wrote:
>> On Wed May 28, 2025 at 9:09 PM IST, Andrew Davis wrote:
>>> On 5/8/25 6:37 AM, Anshul Dalal wrote:
>>>> On Thu May 8, 2025 at 9:32 AM IST, Vignesh Raghavendra wrote:
>>>>>
>>>>>
>>>>> On 5/8/2025 8:42 AM, Anshul Dalal wrote:
>>>>>> On Wed May 7, 2025 at 11:36 PM IST, Andrew Davis wrote:
>>>>>>> On 5/6/25 10:33 PM, Anshul Dalal wrote:
>>>>>>>> On Tue May 6, 2025 at 8:03 PM IST, Andrew Davis wrote:
>>>>>>>>> On 4/28/25 9:12 AM, Anshul Dalal wrote:
>>>>>>>>>> Falcon mode was disabled for TI_SECURE_DEVICE at commit e95b9b4437bc
>>>>>>>>>> ("ti_armv7_common: Disable Falcon Mode on HS devices") for older 32-bit
>>>>>>>>>> HS devices and can be enabled on K3 devices.
>>>>>>>>>>
>>>>>>>>>> For secure boot, the kernel with x509 headers can be packaged in a fit
>>>>>>>>> "can be", this is the issue. Security is not just allowing methods that
>>>>>>>>> are security checked, but forcing the use of such methods. Setting
>>>>>>>>> OS_BOOT opens up several paths that look for non-FIT images. These
>>>>>>>>> images do not enforce authentication like FIT does. This means one can
>>>>>>>>> bypass secure boot when OS_BOOT is enabled by simply placing a non-FIT
>>>>>>>>> boot image on the boot media.
>>>>>>>>>
>>>>>>>> As per spl_load_image_ext_os, the SPL first tries to load the file set
>>>>>>>> in falcon_args_file env variable but since it's not set in our case. And
>>>>>>>> the only way to set them is by rebuilding u-boot as uEnv.txt is not
>>>>>>>> supported at SPL stage.
>>>>>>>>
>>>>>>>> This means the SPL only loads CONFIG_SPL_FS_LOAD_ARGS_NAME and
>>>>>>>> CONFIG_SPL_FS_LOAD_KERNEL_NAME which are set as the DTB and fitImage
>>>>>>> What is stopping me from replacing the content of the file "fitImage"
>>>>>>> with a normal kernel image? When loading that image the file type
>>>>>>> will be detected as a normal kernel image and all FIT logic bypassed,
>>>>>>> including authentication, breaking our secure chain of trust.
>>>>>>>
>>>>>>> Andrew
>>>>>> That would require booti_setup to be executed in spl_parse_image_header,
>>>>>> which is not possible on the R5 SPL since the required config symbol
>>>>>> CMD_BOOTI is only available for ARM64 platforms.
>>>>>>
>>>>>> In the worst case we end up loading a 32-bit zImage which wouldn't
>>>>>> boot on the Cortex-A cores anyway and would additionally require
>>>>>> enabling CMD_BOOTZ (currently disabled) at build time.
>>>>>
>>>>> Is there any path where R5 SPL can be tricked to load and jump to
>>>>> arbitrary binary? zImage, Image, elf, bin whatever?
>>>>>
>>>>> IOW, does SPL_OS_BOOT always check for some sort of header for the image
>>>>> that it loads and the only type of header we have enabled here is fitImage?
>>>>
>>>> It does check for the header and proceeds only with the desired security
>>>> enforced execution flow if the loaded image is FIT. For all other image
>>>> types, they are guarded by config symbols that are set unset in our case
>>>
>>> Are you sure?
>>>
>>> The only thing preventing this from continuing in spl_parse_image_header()
>>> is a check for CONFIG_SPL_PANIC_ON_RAW_IMAGE. Which is not set for us.
>>>
>>> After that we check if OS_BOOT is enabled and if so allow for kernel
>>> images to also pass[0]. What stops this from functioning?
>>>
>>> Andrew
>>>
>>> [0] https://github.com/u-boot/u-boot/blob/master/common/spl/spl.c#L338
>>>
>> 
>> It would not function because of the unset CONFIG_CMD_BOOTI which can
>> only be set on 64 bit platforms anyway[1]. Hence the following check
>> would fail in spl_parse_image_header:
>> 
>> if (CONFIG_IS_ENABLED(OS_BOOT) && IS_ENABLED(CONFIG_CMD_BOOTI))
>> 
>
> I linked the wrong line, the line a couple below for CONFIG_CMD_BOOTZ
> is the one that concerns me.
>
>> And as I said previously in the thread[2]; worst case is we load a
>> 32-bit zImage, support for which would have to be explicitly enabled at
>> build time as the respective config CMD_BOOTZ is kept unset currently.
>> 
>
> What forces CMD_BOOTZ to not be set? Can it be enabled in SPL at all?
> If it can you should make it "depends on !TI_SECURE_DEVICE" like other
> dangerous to the secure chain of trust configs.
>

Sure, I'll update Kconfig to make CMD_BOOTZ and TI_SECURE_DEVICE
mutually exclusive in the next revision just like SPL_RAW_IMAGE_SUPPORT.

Thanks for the review!

~ Anshul