Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] The driver enqueues a `BringUp` work item to the system workqueue but does not save a handle to it, making it impossible to cancel during `disconnect()`. - [Medium] Overly restrictive Kconfig dependency (`depends on USB = y`) prevents building the driver as a module when the USB subsystem is modular. --
commit c4faf8ad941311cd06fad312c6e36a39253115df Author: Mike Lothian <[email protected]> drm/vino: add DisplayLink DL3 dock skeleton and plaintext bring-up Adds a clean-room, in-kernel Rust DRM driver for DisplayLink DL3 USB docks (Dell Universal Dock D6000). This first patch registers a usb::Driver, binds the control interface, and runs the plaintext connect handshake on a workqueue. > diff --git a/drivers/gpu/drm/vino/Kconfig b/drivers/gpu/drm/vino/Kconfig > new file mode 100644 > index 0000000000000..234ce92736e43 > --- /dev/null > +++ b/drivers/gpu/drm/vino/Kconfig > @@ -0,0 +1,21 @@ > +# SPDX-License-Identifier: GPL-2.0 > +config DRM_VINO > + tristate "DisplayLink DL3 (Vino) open driver" > + depends on USB = y [Severity: Medium] Does this strict dependency prevent the driver from being built as a module on standard distribution kernels? Using "depends on USB" instead of "depends on USB = y" would allow this module to be compiled when the USB subsystem is also built as a module. [ ... ] > diff --git a/drivers/gpu/drm/vino/vino.rs b/drivers/gpu/drm/vino/vino.rs > new file mode 100644 > index 0000000000000..79f446041b648 > --- /dev/null > +++ b/drivers/gpu/drm/vino/vino.rs [ ... ] > + let intf_ref: ARef<usb::Interface> = intf.into(); > + match BringUp::new(intf_ref.clone()) { > + Ok(work) => { > + let _ = workqueue::system().enqueue(work); [Severity: High] Does this code leak the enqueued work item handle? Because the returned handle isn't stored in the driver's struct, it becomes impossible to cancel this work item later during device disconnection. > + dev_info!(cdev, "vino: bring-up queued on system > workqueue\n"); > + } > + Err(e) => dev_info!(cdev, "vino: failed to queue bring-up > ({e:?}) -- WIP\n"), > + } > + > + Ok(Self { _intf: intf_ref }) > + } > + > + fn disconnect<'bound>(intf: &'bound usb::Interface<Core<'_>>, _data: > Pin<&Self>) { > + let dev: &device::Device<Core<'_>> = intf.as_ref(); > + dev_info!(dev, "vino: D6000 disconnected\n"); > + } [Severity: High] Can this lead to a use-after-free panic if the module is unloaded while the work item is still pending? Since the work item isn't flushed or canceled here in disconnect(), the workqueue might attempt to execute the work item's run function after the module's code segment has been unmapped. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
