Re: [PATCH v4 13/13] gpu: nova-core: store Fsp instance in Gpu

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] DMA Use-After-Free due to freeing buffers while GSP is still active on timeout
--

commit ad8609a0c320b89583cef07a17ca5ce5c85167ea
Author: Alexandre Courbot <acourbot@nvidia.com>

gpu: nova-core: store Fsp instance in Gpu

The Fsp instance was only used in the Hopper+ boot path, and
consequently built locally (and immediately dropped) in it.

This worked well as a temporary measure, but the FSP is a GPU
sub-device, so its lifetime should match the GPU rather than a single
boot invocation.

> diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs b/drivers/gpu/nova-core/gsp/hal/gh100.rs
> index 2805a35abb798..de786871c8ec4 100644
> --- a/drivers/gpu/nova-core/gsp/hal/gh100.rs
> +++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs
[ ... ]
> @@ -150,7 +146,7 @@ fn boot(
>              KBox::new(FspUnloadBundle, GFP_KERNEL)? as KBox<dyn UnloadBundle>
>          );
>  
> -        let mut fsp = Fsp::wait_secure_boot(dev, bar, chipset)?;
> +        let fsp = ctx.fsp.as_mut().ok_or(ENODEV)?;
>  
>          let args = FmcBootArgs::new(
>              dev,

[Severity: Critical]
This is a pre-existing issue, but I wanted to ask about the error handling
in this function and how it affects DMA buffers.

If wait_for_gsp_lockdown_release() times out later in Gh100::boot(), the ?
operator causes an early return:

        let res = fsp.boot_fmc(dev, fb_layout, &args);

        wait_for_gsp_lockdown_release(dev, gsp_falcon, args.boot_params_dma_handle())?;

When this happens, args is dropped, and the error propagates back to
Gsp::boot(), which bypasses the unload_guard creation and immediately
drops wpr_meta:

        let unload_bundle = hal
            .boot(&self, &mut ctx, &fb_layout, &wpr_meta)?

Dropping these structures frees their underlying DMA memory. However, since
the timeout occurred, the GSP hardware is not forcefully halted.

A similar issue appears during the unload path in FspUnloadBundle::run().
If a timeout occurs while waiting for RISC-V to halt, the code explicitly
detects that the engine is still active but returns an error without
forcefully halting it:

        read_poll_timeout(
            || Ok(ctx.gsp_falcon.is_riscv_active()),
            |&active| !active,
            Delta::from_millis(10),
            Delta::from_secs(5),
        )
        .map(|_| ())
        .inspect_err(|_| dev_err!(ctx.dev(), "GSP falcon failed to halt\n"))

Since the error propagates and allows the DMA memory to be freed while the
engine continues executing, could the active GSP hardware write to the freed
host memory and cause memory corruption?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260629-nova-bootcontext-v4-0-5539d8469590@nvidia.com?part=13