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

Pre-existing issues:
- [High] Heap buffer overflow due to integer underflow when parsing GSP RPC 
messages.
--

commit 87c0818f37bba3a580cd4e8905cc4819a930a1c7
Author: Hongling Zeng <[email protected]>

nouveau/gsp/rm/alloc: Cleanup IS_ERR_OR_NULL usage

This commit cleans up r535_gsp_rpc_rm_free() by replacing IS_ERR_OR_NULL()
with IS_ERR(). This matches the behavior of nvkm_gsp_rpc_get() which only
returns valid pointers or error pointers, never NULL.

> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c 
> b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
> index 46e3a29f2ad7f..c9f86c0e9b255 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r535/alloc.c

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the RPC
parsing code, could the GSP message queue handling be vulnerable to an
integer underflow?

In r535_gsp_msgq_recv() in rpc.c, the firmware-provided rpc->length is read
from the untrusted GSP shared memory message queue:

r535_gsp_msgq_recv() {
        ...
        rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), info.retries);
        if (IS_ERR(rpc)) {
                kvfree(buf);
                return rpc;
        }

        info.gsp_rpc_len = rpc->length;
        info.continuation = true;
        ...
}

If this length is not validated to be at least sizeof(struct nvfw_gsp_rpc),
it appears we could underflow the length calculation in
r535_gsp_msgq_recv_one_elem() when processing continuation messages:

r535_gsp_msgq_recv_one_elem() {
        ...
        len = min_t(u32, expected, len);

        if (info->continuation)
                memcpy(buf, mqe->data + sizeof(struct nvfw_gsp_rpc),
                       len - sizeof(struct nvfw_gsp_rpc));
        ...
}

If rpc->length (and therefore len) is less than sizeof(struct nvfw_gsp_rpc),
does len - sizeof(struct nvfw_gsp_rpc) underflow to a massive positive value,
leading to an out-of-bounds read and a massive heap buffer overflow into
buf during the memcpy?

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

Reply via email to