Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] The `NovaCore` struct fields are dropped in the wrong order, causing the auxiliary device to be unregistered after the GPU and BAR are destroyed. -- commit 10c5ae1aadef16818d89098db4af7d31890aea8c Author: Vladislav Zaharov <[email protected]> gpu: nova-core: build the debugfs guard before registering the driver This commit reorders the initialization in try_pin_init! so that the debugfs guard is built before the driver registration. This ensures that if initialization fails, the debugfs directory is properly cleaned up instead of persisting across module reloads. The drop order for the module remains unchanged. > diff --git a/drivers/gpu/nova-core/nova_core.rs > b/drivers/gpu/nova-core/nova_core.rs > index 1133c6ce5c554..11fe1d2858a9c 100644 > --- a/drivers/gpu/nova-core/nova_core.rs > +++ b/drivers/gpu/nova-core/nova_core.rs > @@ -47,7 +47,8 @@ fn drop(&mut self) { > #[pin_data] > struct NovaCoreModule { > // Fields are dropped in declaration order, so `_driver` is dropped > first, > - // then `_debugfs_guard` clears `DEBUGFS_ROOT`. > + // then `_debugfs_guard` clears `DEBUGFS_ROOT`. They are initialized the > + // other way round, see `init()`. > #[pin] > _driver: Registration<pci::Adapter<driver::NovaCoreDriver>>, > _debugfs_guard: DebugfsRootGuard, [Severity: High] This isn't a bug introduced by this patch, but since this change discusses drop orders, does the struct declaration order of NovaCore in drivers/gpu/nova-core/driver.rs cause the auxiliary device to be unregistered after the GPU and BAR are destroyed? drivers/gpu/nova-core/driver.rs: #[pin_data] pub(crate) struct NovaCore<'bound> { #[pin] pub(crate) gpu: Gpu<'bound>, bar: pci::Bar<'bound, BAR0_SIZE>, #[allow(clippy::type_complexity)] _reg: auxiliary::Registration<'bound, CovariantForLt!(())>, } In Rust, struct fields are dropped in declaration order (first to last). In NovaCore, gpu and bar are declared before _reg. Therefore, the GPU is dropped and the MMIO region is unmapped before the auxiliary device registration (_reg) is dropped. When _reg is subsequently dropped, it synchronously unregisters the child device and invokes the child driver's (e.g., nova-drm) remove callback. If the child driver attempts to access the GPU or MMIO BAR during its teardown (e.g., to disable the display or flush buffers), will it access destroyed resources or unmapped memory, leading to a UAF and kernel oops? This deterministic crash would trigger whenever the NovaCore device is unbound or the module is unloaded. [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
