Re: [PATCH 5/7] drm/tegra: falcon: Add support for RISC-V external boot

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Missing size validation for the `.desc` firmware file allows an out-of-bounds read of the firmware descriptor buffer.
- [High] Inverted condition in `falcon_wait_idle()` for RISC-V incorrectly waits for the CPU to become active rather than idle, causing premature return.
- [High] Firmware segment offsets and sizes from the `.desc` file are not validated against the main firmware buffer size, leading to out-of-bounds DMA reads.
- [Medium] Missing endianness conversion when reading `u32` fields from the `.desc` firmware descriptor breaks on big-endian architectures.
--

commit 9b921eae84dc7cacf7c324bd76e166d6a5d942be
Author: Mikko Perttunen <mperttunen@nvidia.com>

drm/tegra: falcon: Add support for RISC-V external boot

This commit adds support for loading and booting RISC-V firmwares on Falcons
with RISC-V hardware. It introduces a new firmware layout and uses different
registers for the boot flow compared to traditional Falcons.

> diff --git a/drivers/gpu/drm/tegra/falcon.c b/drivers/gpu/drm/tegra/falcon.c
> index 17f616bbcb45b..1172356b6af3b 100644
> --- a/drivers/gpu/drm/tegra/falcon.c
> +++ b/drivers/gpu/drm/tegra/falcon.c
> @@ -26,8 +26,12 @@ int falcon_wait_idle(struct falcon *falcon)
>  {
>  	u32 value;
>  
> -	return readl_poll_timeout(falcon->regs + FALCON_IDLESTATE, value,
> -				  (value == 0), 10, 100000);
> +	if (falcon->riscv)
> +		return readl_poll_timeout(falcon->regs + RISCV_CPUCTL, value,
> +					  (value & RISCV_CPUCTL_ACTIVE_STAT_ACTIVE), 10, 100000);

[Severity: High]
Does this condition successfully terminate when the CPU is active rather than
idle?

If falcon_boot() calls this immediately after issuing the STARTCPU command,
the processor will instantly become active, and the wait will return
immediately instead of waiting for the processor to go idle. Would negating
the condition to !(value & RISCV_CPUCTL_ACTIVE_STAT_ACTIVE) correctly wait
for the firmware boot sequence to finish?

> +	else
> +		return readl_poll_timeout(falcon->regs + FALCON_IDLESTATE, value,
> +					  (value == 0), 10, 100000);
>  }

[ ... ]

> @@ -122,6 +126,17 @@ static int falcon_parse_firmware_image(struct falcon *falcon)
>  	return 0;
>  }
>  
> +static void falcon_parse_firmware_desc(struct falcon *falcon)
> +{
> +	struct falcon_fw_riscv_desc *desc =
> +		(struct falcon_fw_riscv_desc *)falcon->firmware.desc_firmware->data;

[Severity: High]
Is it possible for the loaded firmware file to be smaller than
sizeof(struct falcon_fw_riscv_desc)?

If a truncated or malformed .desc file is loaded from userspace, could
casting and reading desc_firmware->data here result in an out-of-bounds
memory read of the allocated buffer?

> +
> +	falcon->firmware.code.offset = desc->code_offset;
> +	falcon->firmware.code.size = desc->code_size;
> +	falcon->firmware.data.offset = desc->data_offset;
> +	falcon->firmware.data.size = desc->data_size;

[Severity: Medium]
Do these assignments need endianness conversion, such as le32_to_cpu()?

The older .bin parsing path safely copies and byte-swaps the firmware via
falcon_copy_firmware_image(). Since this RISC-V path appears to perform
no conversions, could reading these fields directly from the raw byte
array produce incorrect, byte-swapped values on big-endian architectures?

[Severity: High]
Are these offsets and sizes validated against the main firmware buffer size?

In falcon_boot(), these sizes are used to calculate bounds and program
the Falcon hardware DMA engine via falcon_copy_chunk():

falcon_boot() {
    ...
    for (offset = 0; offset < falcon->firmware.data.size; offset += 256)
        falcon_copy_chunk(falcon,
                          falcon->firmware.data.offset + offset,
                          offset, FALCON_MEMORY_DATA);
    ...
}

If the values are not checked to ensure
offset + size <= falcon->firmware.size, could a maliciously crafted .desc
file instruct the DMA controller to read out-of-bounds physical memory?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-t264-host1x-v1-0-8d934987de67@nvidia.com?part=5