Re: [PATCH v9 2/2] rust: serio: add serio bus abstractions and port Gravis Stinger driver

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] rust: serio: non-existent bindings::irqreturn::IRQ_* paths
  break build
- [Medium] rust: serio: unsound Device<Bound> cast in
  Registration::connect_callback()
--

commit 8b9bc178d0ee71cd5694b16b76519c8a7ef9200e
Author: Vecheslav Filatov <harepro29@gmail.com>

rust: serio: add serio bus abstractions and port Gravis Stinger driver

This commit adds Rust abstractions for the serio bus subsystem and ports
the Gravis Stinger serial gamepad driver to Rust.

> diff --git a/rust/kernel/serio.rs b/rust/kernel/serio.rs
> new file mode 100644
> index 0000000000000..4e16917ddd70c
> --- /dev/null
> +++ b/rust/kernel/serio.rs
> @@ -0,0 +1,336 @@

[ ... ]

> +pub use crate::irq::IrqReturn;
> +
> +#[allow(non_upper_case_globals)]
> +const irqreturn_IRQ_NONE: bindings::irqreturn_t = bindings::irqreturn::IRQ_NONE;
> +#[allow(non_upper_case_globals)]
> +const irqreturn_IRQ_HANDLED: bindings::irqreturn_t = bindings::irqreturn::IRQ_HANDLED;

[Severity: High]
Does this compile when CONFIG_RUST=y and CONFIG_SERIO=y are enabled?

bindgen generates bindings::irqreturn as a c_uint type alias with top-level
constants bindings::irqreturn_IRQ_NONE and bindings::irqreturn_IRQ_HANDLED
(as used in rust/kernel/irq/request.rs), so bindings::irqreturn::IRQ_NONE
and bindings::irqreturn::IRQ_HANDLED fail to resolve with E0599.  Dropping
these local constants and returning bindings::irqreturn_IRQ_NONE and
bindings::irqreturn_IRQ_HANDLED directly in
Registration::interrupt_callback() avoids the build failure.

[ ... ]

> +/// Trait implemented by serio device drivers.
> +pub trait Driver: Send + Sync + Sized {
> +    /// Context data stored per connected device instance.
> +    type Data: ForeignOwnable + Send + Sync;
> +
> +    /// Pending resources allocated during probe to be published after the port opens.
> +    type Pending;
> +
> +    /// Published device handle created after the port opens.
> +    type Published;
> +
> +    /// Device ID match table.
> +    const ID_TABLE: IdTable;
> +
> +    /// Initializes driver data before the port is opened.
> +    fn init(dev: &Device<device::Bound>) -> Result<(Self::Data, Self::Pending)>;

[ ... ]

> +    unsafe extern "C" fn connect_callback(
> +        serio: *mut bindings::serio,
> +        drv: *mut bindings::serio_driver,
> +    ) -> c_int {
> +        if serio.is_null() {
> +            return crate::error::code::EINVAL.to_errno();
> +        }
> +
> +        // SAFETY: `serio` is valid. `Device<Bound>` is repr(transparent).
> +        let dev = unsafe { &*(serio.cast::<Device<device::Bound>>()) };
> +
> +        // 1. Initialize driver data before port is opened.
> +        let (data, pending) = match T::init(dev) {

[Severity: Medium]
Is it sound to pass &Device<device::Bound> to T::init() here?

Because Device<Ctx> implements AsRef<device::Device<Ctx>>, safe code in
Driver::init() can obtain &device::Device<device::Bound> and register
managed resources via Devres::new() or devres::register().  However, manual
driver binding through sysfs drvctl calls serio_bind_driver() outside
really_probe():

drivers/input/serio/serio.c:serio_bind_driver() {
	...
		serio->dev.driver = &drv->driver;
		if (serio_connect_driver(serio, drv)) {
			serio->dev.driver = NULL;
			return -ENODEV;
		}

		err = device_bind_driver(&serio->dev);
		if (err) {
			serio_disconnect_driver(serio);
			serio->dev.driver = NULL;
			return err;
		}
	...
}

If connect_callback() or device_bind_driver() fails in that path,
devres_release_all() is never called on &serio->dev, leaving any Devres
registered during T::init() unrevoked on an unbound device and causing
later really_probe() attempts on that port to fail with -EBUSY.  Passing
&Device<device::Normal> (or &Device) to Driver::init() prevents callers
from registering devres resources before binding completes.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261107102106.546041-1-harepro29@gmail.com?part=2