Re: [PATCH v4 3/5] nouveau/gsp/rm/alloc: Cleanup IS_ERR_OR_NULL usage

[sashiko-bot@kernel.org]

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 <zenghongling@kylinos.cn>

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/20260622035454.28003-1-zenghongling@kylinos.cn?part=3