Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:

Pre-existing issues:
- [High] Integer overflow in `panthor_fw_read_build_info` leads to an 
out-of-bounds read when parsing untrusted firmware.
--

commit 36848cd8008e907cd2d324339568a4a41f683911
Author: Boris Brezillon <[email protected]>

drm/panthor: Drop a needless check in panthor_fw_unplug()

panthor_fw_unplug() is only called if we at least managed to initialize
the IRQ, so it's safe to drop the "is IRQ initialized" check.

> diff --git a/drivers/gpu/drm/panthor/panthor_fw.c 
> b/drivers/gpu/drm/panthor/panthor_fw.c
> index 986151681b246..4fbddb9e18c8b 100644
> --- a/drivers/gpu/drm/panthor/panthor_fw.c
> +++ b/drivers/gpu/drm/panthor/panthor_fw.c

[ ... ]

drivers/gpu/drm/panthor/panthor_fw.c:panthor_fw_read_build_info() {
    ...
        if (hdr.meta_start > fw->size ||
            hdr.meta_start + hdr.meta_size > fw->size) {
                drm_err(&ptdev->base, "Firmware build info corrupt\n");
                /* We don't need the build info, so continue */
                return 0;
        }

        if (memcmp(git_sha_header, fw->data + hdr.meta_start, header_len)) {
                /* Not the expected header, this isn't metadata we understand */
                return 0;
        }

        /* Check that the git SHA is NULL terminated as expected */
        if (fw->data[hdr.meta_start + hdr.meta_size - 1] != '\0') {
    ...
}

[Severity: High]
This is a pre-existing issue, but does this bounds check use 32-bit
arithmetic that can wrap around before the promotion to size_t?

If an attacker supplies a large meta_size (e.g., 0xFFFFFFFF) and a carefully
chosen meta_start (e.g., 0) such that their sum wraps around, could this
bypass the check since 0 > fw->size evaluates to false?

Execution would then call memcmp() on the attacker-controlled meta_start
offset. If the string matches "git_sha: ", execution proceeds to:

    fw->data[hdr.meta_start + hdr.meta_size - 1]

Could this evaluate to fw->data[0xFFFFFFFF] (a 4GB out-of-bounds offset)
and cause a page fault or oops when parsing untrusted firmware?

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

Reply via email to