Re: [PATCH 5/5] gpu: nova-core: falcon: Move PFSP register

[sashiko-bot@kernel.org]

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

New issues:
- [Low] The commit modifies import statements without migrating them to the kernel's vertical import style.

Pre-existing issues:
- [High] FSP queue registers are defined as arrays of 8 elements without a `stride` parameter, defaulting to a 4-byte stride which silently overlaps with adjacent registers.
- [High] Unbounded allocation in `recv_msg` based on untrusted hardware input can trigger an Out-of-Memory (OOM) condition or kernel panic.
--

commit 2170d35504c32c252c52eca31b351aad8e873b56
Author: Antonin Malzieu Ridolfi <dev@nanonej.com>

gpu: nova-core: falcon: Move PFSP register

This patch moves the PFSP register definitions into the falcon module and
updates the registers' visibility.

> diff --git a/drivers/gpu/nova-core/falcon/fsp.rs b/drivers/gpu/nova-core/falcon/fsp.rs
> index 52cdb84ef0e80..547843e6483c6 100644
> --- a/drivers/gpu/nova-core/falcon/fsp.rs
> +++ b/drivers/gpu/nova-core/falcon/fsp.rs
> @@ -26,10 +26,10 @@
>          Falcon,
>          FalconEngine,
>          PFalcon2Base,
> -        PFalconBase, //
> +        PFalconBase,
> +        regs, //
>      },
> -    num,
> -    regs, //
> +    num, //
>  };

[Severity: Low]
Does this import block need to be updated to match the kernel vertical import
style? The Rust subsystem coding guidelines require flattening nested imports
when they are modified.

[ ... ]

> diff --git a/drivers/gpu/nova-core/falcon/regs.rs b/drivers/gpu/nova-core/falcon/regs.rs
> index 4d4e69b235fc6..81530abfb54c3 100644
> --- a/drivers/gpu/nova-core/falcon/regs.rs
> +++ b/drivers/gpu/nova-core/falcon/regs.rs

[ ... ]

> +register! {
> +    pub(super) NV_PFSP_QUEUE_HEAD(u32)[8] @ 0x008f2c00 {
> +        31:0    address => u32;
> +    }
> +
> +    pub(super) NV_PFSP_QUEUE_TAIL(u32)[8] @ 0x008f2c04 {
> +        31:0    address => u32;
> +    }

[Severity: High]
This is a pre-existing issue, but does NV_PFSP_QUEUE_HEAD need a stride
parameter?

Since the elements are 4 bytes, omitting the stride defaults it to 4 bytes.
This would place index 1 of NV_PFSP_QUEUE_HEAD at 0x008f2c04, which overlaps
with NV_PFSP_QUEUE_TAIL. 

Should this be defined as [8, stride = 8] to prevent accessing the wrong
register for indices > 0?

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but is there an unbounded
allocation risk in recv_msg()?

Looking at fsp.rs, poll_msgq() derives the message size directly from
the hardware MMIO registers without bounds checking:

tail.saturating_sub(head).saturating_add(4)

This untrusted size is then used to allocate kernel memory:

drivers/gpu/nova-core/falcon/fsp.rs:recv_msg() {
    ...
    let mut buffer = KVec::<u8>::new();
    buffer.resize(msg_size, 0, GFP_KERNEL)?;
    ...
}

Could a compromised or misbehaving FSP firmware provide a massive message
size, exhausting kernel memory and triggering an OOM condition?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260617-nova-core-regs-split-v1-0-4c7dc4450ea7@nanonej.com?part=5