Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [Critical] Error paths in vic_load_firmware leave a dangling pointer in vic->falcon.firmware.virt, causing Use-After-Free and Double Free. -- commit c58aa87e57c2ab3ec5383df99d29ec8f6a79e162 Author: Mikko Perttunen <[email protected]> Subject: drm/tegra: vic: Add Tegra264 support Add support for VIC on Tegra264. The Tegra264 VIC uses a RISC-V based Falcon microcontroller instead of the traditional Falcon previously, and has the TRANSCFG register in a different place. > diff --git a/drivers/gpu/drm/tegra/vic.c b/drivers/gpu/drm/tegra/vic.c > index 332c9b563d3f4..46c7e00de3473 100644 > --- a/drivers/gpu/drm/tegra/vic.c > +++ b/drivers/gpu/drm/tegra/vic.c [ ... ] > @@ -302,7 +330,6 @@ static int vic_load_firmware(struct vic *vic) > return err; > } > [Severity: Critical] This is a pre-existing issue, but I noticed a potential use-after-free and double-free in the error handling of vic_load_firmware(). When memory allocation succeeds, the pointers are assigned: vic->falcon.firmware.virt = virt; vic->falcon.firmware.iova = iova; If a subsequent step fails (such as falcon_load_firmware() or dma_map_single()), the code jumps to the cleanup label, frees the memory, but does not clear the pointer: cleanup: if (!client->group) dma_free_coherent(vic->dev, size, virt, iova); else tegra_drm_free(tegra, size, virt, iova); mutex_unlock(&lock); return err; If vic_load_firmware() is called again (for example, during the next PM runtime resume), it sees the non-NULL pointer, assumes the firmware is loaded, and returns 0. Will this cause the driver to program the hardware to DMA from the freed memory? Furthermore, when the device is unbound, will the teardown path unconditionally free vic->falcon.firmware.virt again causing a double-free? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
