Re: [RFC PATCH 1/7] drm/vino: add DisplayLink DL3 dock skeleton and plaintext bring-up

[sashiko-bot@kernel.org]

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 <mike@fireburn.co.uk>

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/20260617151249.2937-1-mike@fireburn.co.uk?part=1