Re: [PATCH v3 1/7] gpu: nova-core: Add public driver API to nova-core

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Forging a `'bound` lifetime for a local field reference inside a `KBox` creates a reference that outlives its referent and violates strict aliasing.
--

--- Patch [1]: [PATCH v3 1/7] gpu: nova-core: Add public driver API to nova-core ---
commit 75bdf4e26c67bf993498d459d03f978972444242
Author: Alistair Popple <apopple@nvidia.com>

gpu: nova-core: Add public driver API to nova-core

Nova core will be used to export core functionality to other drivers
which will bind to it via auxiliary bus devices. Add a NovaCoreApi type
which drivers can use to call nova-core methods.

> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> --- a/drivers/gpu/nova-core/driver.rs
> +++ b/drivers/gpu/nova-core/driver.rs

[ ... ]

> @@ -86,15 +90,33 @@ fn probe<'bound>(
>                  // (`try_pin_init!()` initializes fields in declaration order), lives at a pinned
>                  // stable address, and is dropped after `gpu` (struct field drop order).
>                  gpu <- Gpu::new(pdev, unsafe { &*core::ptr::from_ref(bar) }),
> -                _reg: auxiliary::Registration::new(
> -                    pdev.as_ref(),
> -                    c"nova-drm",
> -                    // TODO[XARR]: Use XArray or perhaps IDA for proper ID allocation/recycling. For
> -                    // now, use a simple atomic counter that never recycles IDs.
> -                    AUXILIARY_ID_COUNTER.fetch_add(1, Relaxed),
> -                    crate::MODULE_NAME,
> -                    (),
> -                )?,
> +
> +                _reg: {
> +                    // TODO: Use `&gpu` self-referential pin-init syntax once available.
> +                    //
> +                    // SAFETY: `gpu` is initialized before this expression is evaluated
> +                    // (`try_pin_init!()` initializes fields in declaration order), lives at
> +                    // a pinned stable address, and is dropped after `_reg` (struct field
> +                    // drop order).
> +                    let gpu = unsafe {
> +                        Pin::new_unchecked(&*core::ptr::from_ref(gpu.as_ref().get_ref()))
> +                    };

[Severity: High]
Does creating this reference to gpu violate Rust's strict aliasing rules?

Because NovaCore is dynamically allocated in a KBox during device probe,
expanding the lifetime of the local gpu field reference to the longer 'bound
lifetime appears unsafe. The KBox does not strictly outlive 'bound since it
is dropped when the driver unbinds, which could eventually leave child
drivers holding a dangling reference.

Furthermore, when the pin_init macro completes and forms the KBox, the
underlying Box asserts unique ownership over the memory. If the previously
created reference to gpu has escaped into the externally allocated
registration data, it might be invalidated immediately under Stacked Borrows
or Tree Borrows. This could potentially lead to undefined behavior when the
reference is accessed later by the child driver.

> +
> +                    // SAFETY: `NovaCore` is dropped when the device is unbound;
> +                    // i.e. `mem::forget()` is never called on it.
> +                    unsafe {
> +                        auxiliary::Registration::new_with_lt(
> +                            pdev.as_ref(),
> +                            c"nova-drm",

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260723063046.1265791-1-apopple@nvidia.com?part=1