Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Missing bounds check, endianness conversion, and size validation of 
the RISC-V firmware descriptor leads to out-of-bounds memory accesses and 
unbounded DMA.
--

commit ce83e1285215f11b260dbf628363fe352ce09b78
Author: Mikko Perttunen <[email protected]>

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

Add support for loading and booting RISC-V firmwares on Falcons with
RISC-V hardware.

> 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

[ ... ]

> @@ -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;
> +
> +     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: High]
Does this code need to validate the size of desc_firmware before casting its
data to struct falcon_fw_riscv_desc? If the firmware file is smaller than the
struct size, this could lead to an out-of-bounds read.

Also, do these fields need le32_to_cpu() conversion for big-endian systems,
similar to how the main firmware is handled in falcon_copy_firmware_image()?

Finally, are the parsed sizes and offsets validated against the main firmware's
size? If not, these unchecked values could cause out-of-bounds host memory
accesses later in falcon_boot() when they are used in the falcon_copy_chunk()
DMA loops:

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

> +}
> +
>  int falcon_read_firmware(struct falcon *falcon, const char *name)
>  {

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=5

Reply via email to