* [PATCH v2 0/2] rust: input: add input/serio abstractions and port Gravis Stinger driver
@ 2026-11-06 20:14 clicedvly768
2026-11-06 20:14 ` [PATCH v2 1/2] rust: input: add input device abstractions clicedvly768
2026-11-06 20:14 ` [PATCH v2 2/2] rust: serio: add serio bus abstractions and port Gravis Stinger driver clicedvly768
0 siblings, 2 replies; 5+ messages in thread
From: clicedvly768 @ 2026-11-06 20:14 UTC (permalink / raw)
To: ojeda, dmitry.torokhov; +Cc: rust-for-linux, linux-input, clicedvly768
This series introduces safe Rust abstractions for the Linux input and
serio (Serial I/O) subsystems and demonstrates their real-world usage
by porting the legacy Gravis Stinger serial gamepad driver to safe Rust.
Following community review, the serio bus abstractions and the Stinger
driver port have been consolidated into the second patch, while keeping
the input device abstractions in the first patch.
Motivation and Design:
- In the Rust for Linux (RfL) philosophy, device drivers should not rely
on raw FFI and unsafe blocks. Instead, the kernel crate provides safe,
idiomatic Rust abstractions.
- The Gravis Stinger gamepad is an ideal candidate for demonstrating safe
Rust driver development in the input subsystem: it uses a well-defined
4-byte serial protocol at 9600 baud, 10 buttons, and 2 analog axes.
- The ported driver (`drivers/input/joystick/stinger_rust.rs`) contains
ZERO unsafe blocks, using RAII types (`AllocatedDevice`,
`RegisteredDevice`, `serio::Registration`) for memory safety and
device lifetime management.
- Non-regression: `CONFIG_JOYSTICK_STINGER_RUST` is offered alongside
the original C driver (`stinger.c`), ensuring architectures without
Rust (e.g. powerpc defconfigs) suffer no hardware support regressions.
Testing:
- Built with Clang/LLVM and rustc 1.85.
- Passes all strict kernel Rust checks: `make LLVM=1 rustfmtcheck`,
`make LLVM=1 rustdoc`, and `make LLVM=1 CLIPPY=1` with zero warnings.
- Tested both built-in (`CONFIG_JOYSTICK_STINGER=y`) and loadable module
(`CONFIG_JOYSTICK_STINGER=m`) configurations.
- Tested end-to-end in a custom QEMU virtual test environment with serial
port loopback (`serport` line discipline `N_MOUSE` and `SERIO_STINGER`).
- Verified event reporting (`EV_KEY`, `EV_ABS`, `EV_SYN`) via `evtest`
and an automated test suite across all 10 buttons and
both axes (-64..64).
Changes in v2:
- Consolidated serio abstractions and Stinger driver port into patch 2/2.
- Gated built-in Rust modules and helpers with `#[cfg(CONFIG_* = "y")]`
and `#if IS_REACHABLE(CONFIG_*)` to prevent vmlinux link failures
when CONFIG_INPUT or CONFIG_SERIO is compiled as a module or disabled.
- Retained original `stinger.c` and introduced `JOYSTICK_STINGER_RUST`
to avoid hardware support regressions on architectures without RUST.
- Replaced raw slice `&'static [DeviceId]` with
`kernel::device_id::RawDeviceId` and `kernel::device_id::IdTable`
via `kernel::serio_device_table!`, guaranteeing compile-time
zero-sentinel termination and exporting modpost device aliases.
- Fixed module owner tracking: removed `helpers.o` registration wrappers;
`Registration::new` now takes `&'static ThisModule` and calls
`bindings::__serio_register_driver` with the driver's own module pointer.
- Stored `Opaque<bindings::serio_driver>` in `Pin<KBox<...>>` to avoid
aliasing violations on C-owned memory.
- Fixed return error code sign in `connect_callback` (`to_errno()` is
already negative).
- Reordered port opening: `serio_open` is called before publishing the
input device to userspace.
- Reused `kernel::irq::IrqReturn` instead of duplicating a custom enum.
- Integrated `serio::Device` with generic device infrastructure:
implemented `AsRef<device::Device>` and `Deref<Target = device::Device>`,
and added `AllocatedDevice::set_parent()` in `input.rs` to maintain
the sysfs device hierarchy (`&serio->dev`).
- Switched `AllocatedDevice::set_phys()` to `&'static CStr`.
- Added bounds check `axis < ABS_CNT` in `set_abs_params()`.
- Added `#[inline]` on small helper methods and used vertical imports.
clicedvly768 (2):
rust: input: add input device abstractions
rust: serio: add serio bus abstractions and port Gravis Stinger driver
drivers/input/joystick/Kconfig | 14 ++
drivers/input/joystick/Makefile | 4 +
drivers/input/joystick/stinger_rust.rs | 178 +++++++++++++++++
rust/bindings/bindings_helper.h | 2 +
rust/helpers/helpers.c | 2 +
rust/helpers/input.c | 22 +++
rust/helpers/serio.c | 17 ++
rust/kernel/input.rs | 214 +++++++++++++++++++++
rust/kernel/lib.rs | 4 +
rust/kernel/serio.rs | 256 +++++++++++++++++++++++++
10 files changed, 713 insertions(+)
create mode 100644 drivers/input/joystick/stinger_rust.rs
create mode 100644 rust/helpers/input.c
create mode 100644 rust/helpers/serio.c
create mode 100644 rust/kernel/input.rs
create mode 100644 rust/kernel/serio.rs
--
2.56.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] rust: input: add input device abstractions
2026-11-06 20:14 [PATCH v2 0/2] rust: input: add input/serio abstractions and port Gravis Stinger driver clicedvly768
@ 2026-11-06 20:14 ` clicedvly768
2026-11-06 20:27 ` sashiko-bot
2026-11-06 20:14 ` [PATCH v2 2/2] rust: serio: add serio bus abstractions and port Gravis Stinger driver clicedvly768
1 sibling, 1 reply; 5+ messages in thread
From: clicedvly768 @ 2026-11-06 20:14 UTC (permalink / raw)
To: ojeda, dmitry.torokhov; +Cc: rust-for-linux, linux-input, clicedvly768
Add safe Rust abstractions for Linux input devices (struct input_dev).
This provides:
- `AllocatedDevice`: a newly allocated, unregistered input device handle
that allows configuring capabilities (keys, absolute axes limits, parent
device) before registration.
- Safe string methods: `set_name` and `set_phys` accept `&'static CStr`
to prevent use-after-free since the input core does not copy strings.
- Bounds-checked `set_abs_params` validating `axis < ABS_CNT` (0x40)
to prevent memory corruption in `dev->absinfo`.
- `RegisteredDevice`: a registered input device handle providing safe,
inlined methods to report keys, absolute coordinates, and synchronization
events. Automatically calls `input_unregister_device` on drop (RAII).
- Constants for bus types (`bus::*`) and event types (`event_type::*`).
- Alphabetically sorted imports and module entries gated with
`#[cfg(CONFIG_INPUT = "y")]` and `#if IS_REACHABLE(CONFIG_INPUT)`
to avoid link failures when CONFIG_INPUT is disabled or modular.
Signed-off-by: clicedvly768 <harepro29@gmail.com>
---
rust/bindings/bindings_helper.h | 1 +
rust/helpers/helpers.c | 1 +
rust/helpers/input.c | 22 ++++
rust/kernel/input.rs | 214 ++++++++++++++++++++++++++++++++
rust/kernel/lib.rs | 2 +
5 files changed, 240 insertions(+)
create mode 100644 rust/helpers/input.c
create mode 100644 rust/kernel/input.rs
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 3791af0aadc7..4b09d3980e10 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -64,6 +64,7 @@
#include <linux/fwctl.h>
#include <linux/fs.h>
#include <linux/i2c.h>
+#include <linux/input.h>
#include <linux/interrupt.h>
#include <linux/io-pgtable.h>
#include <linux/ioport.h>
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index d62c7be1ca7b..77cec85ad153 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -67,6 +67,7 @@
#include "fs.c"
#include "fwctl.c"
#include "gpu.c"
+#include "input.c"
#include "interrupt.c"
#include "io.c"
#include "irq.c"
diff --git a/rust/helpers/input.c b/rust/helpers/input.c
new file mode 100644
index 000000000000..f43a06acfdc3
--- /dev/null
+++ b/rust/helpers/input.c
@@ -0,0 +1,22 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/input.h>
+
+#if IS_REACHABLE(CONFIG_INPUT)
+
+__rust_helper void rust_helper_input_report_key(struct input_dev *dev, unsigned int code, int value)
+{
+ input_report_key(dev, code, value);
+}
+
+__rust_helper void rust_helper_input_report_abs(struct input_dev *dev, unsigned int code, int value)
+{
+ input_report_abs(dev, code, value);
+}
+
+__rust_helper void rust_helper_input_sync(struct input_dev *dev)
+{
+ input_sync(dev);
+}
+
+#endif /* IS_REACHABLE(CONFIG_INPUT) */
diff --git a/rust/kernel/input.rs b/rust/kernel/input.rs
new file mode 100644
index 000000000000..bef0c3a07da0
--- /dev/null
+++ b/rust/kernel/input.rs
@@ -0,0 +1,214 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Input subsystem abstractions for safe Rust drivers.
+//!
+//! Provides a safe Rust object-oriented interface around Linux `struct input_dev`,
+//! ensuring safe allocation, capability configuration, registration, and RAII cleanup.
+
+use crate::{
+ bindings, device,
+ error::{
+ code::{EINVAL, ENOMEM},
+ Result,
+ },
+ prelude::*,
+ str::CStr,
+};
+use core::ptr::NonNull;
+
+/// Bus types for input devices.
+pub mod bus {
+ use super::*;
+
+ /// PCI bus.
+ pub const PCI: u16 = bindings::BUS_PCI as u16;
+ /// USB bus.
+ pub const USB: u16 = bindings::BUS_USB as u16;
+ /// RS232 serial bus.
+ pub const RS232: u16 = bindings::BUS_RS232 as u16;
+}
+
+/// Event types reported by input devices.
+pub mod event_type {
+ use super::*;
+
+ /// Key or button press/release events.
+ pub const KEY: u32 = bindings::EV_KEY;
+ /// Absolute axis movement events.
+ pub const ABS: u32 = bindings::EV_ABS;
+}
+
+/// An allocated input device being configured prior to registration.
+pub struct AllocatedDevice {
+ ptr: NonNull<bindings::input_dev>,
+}
+
+impl AllocatedDevice {
+ /// Allocates a new input device.
+ pub fn new() -> Result<Self> {
+ // SAFETY: `input_allocate_device` returns either a valid pointer or NULL.
+ let ptr = unsafe { bindings::input_allocate_device() };
+ let ptr = NonNull::new(ptr).ok_or(ENOMEM)?;
+
+ Ok(Self { ptr })
+ }
+
+ /// Sets the human-readable device name.
+ #[inline]
+ pub fn set_name(&mut self, name: &'static CStr) {
+ // SAFETY: `self.ptr` is valid and `name` has static lifetime.
+ unsafe {
+ (*self.ptr.as_ptr()).name = name.as_char_ptr().cast();
+ }
+ }
+
+ /// Sets the physical location string.
+ ///
+ /// The input core does not copy the string and requires it to remain valid
+ /// throughout the device's lifetime, so a static NUL-terminated C string is required.
+ #[inline]
+ pub fn set_phys(&mut self, phys: &'static CStr) {
+ // SAFETY: `self.ptr` is valid and `phys` has static lifetime.
+ unsafe {
+ (*self.ptr.as_ptr()).phys = phys.as_char_ptr().cast();
+ }
+ }
+
+ /// Sets device identifier (bus type, vendor, product, version).
+ #[inline]
+ pub fn set_id(&mut self, bustype: u16, vendor: u16, product: u16, version: u16) {
+ // SAFETY: `self.ptr` is valid.
+ unsafe {
+ let dev = self.ptr.as_ptr();
+ (*dev).id.bustype = bustype;
+ (*dev).id.vendor = vendor;
+ (*dev).id.product = product;
+ (*dev).id.version = version;
+ }
+ }
+
+ /// Sets the parent device for this input device.
+ #[inline]
+ pub fn set_parent(&mut self, parent: &device::Device) {
+ // SAFETY: `self.ptr` is valid, and `parent.as_raw()` returns a valid `struct device`.
+ unsafe {
+ (*self.ptr.as_ptr()).dev.parent = parent.as_raw();
+ }
+ }
+
+ /// Enables support for a capability (e.g. key or axis).
+ #[inline]
+ pub fn set_capability(&mut self, type_: u32, code: u32) {
+ // SAFETY: `self.ptr` is valid and type/code are valid capability values.
+ unsafe {
+ bindings::input_set_capability(self.ptr.as_ptr(), type_, code);
+ }
+ }
+
+ /// Sets parameter limits for an absolute axis.
+ ///
+ /// Bounds-checks `axis < ABS_CNT` (0x40) to prevent out-of-bounds writes
+ /// to the `absinfo` array in `struct input_dev`.
+ pub fn set_abs_params(
+ &mut self,
+ axis: u32,
+ min: i32,
+ max: i32,
+ fuzz: i32,
+ flat: i32,
+ ) -> Result {
+ if axis >= bindings::ABS_CNT {
+ return Err(EINVAL);
+ }
+
+ // SAFETY: `self.ptr` is valid and `axis` has been verified within bounds.
+ unsafe {
+ bindings::input_set_abs_params(self.ptr.as_ptr(), axis, min, max, fuzz, flat);
+ }
+ Ok(())
+ }
+
+ /// Registers the input device with the kernel input core.
+ ///
+ /// Consumes the `AllocatedDevice` and returns a [`RegisteredDevice`].
+ pub fn register(self) -> Result<RegisteredDevice> {
+ let ptr = self.ptr;
+ // Don't run the AllocatedDevice drop destructor, registration takes ownership.
+ core::mem::forget(self);
+
+ // SAFETY: `ptr` is valid and was allocated by `input_allocate_device`.
+ let ret = unsafe { bindings::input_register_device(ptr.as_ptr()) };
+ if ret != 0 {
+ // SAFETY: Free the allocated device if registration failed.
+ unsafe {
+ bindings::input_free_device(ptr.as_ptr());
+ }
+ return Err(crate::error::Error::from_errno(ret));
+ }
+
+ Ok(RegisteredDevice { ptr })
+ }
+}
+
+impl Drop for AllocatedDevice {
+ fn drop(&mut self) {
+ // SAFETY: `self.ptr` was allocated by `input_allocate_device` and has not been registered.
+ unsafe {
+ bindings::input_free_device(self.ptr.as_ptr());
+ }
+ }
+}
+
+// SAFETY: `AllocatedDevice` exclusively owns its `input_dev` pointer and can be transferred
+// across threads.
+unsafe impl Send for AllocatedDevice {}
+
+/// A registered input device capable of reporting events to userspace.
+pub struct RegisteredDevice {
+ ptr: NonNull<bindings::input_dev>,
+}
+
+impl RegisteredDevice {
+ /// Reports a binary button or key state event.
+ #[inline]
+ pub fn report_key(&self, code: u32, value: bool) {
+ // SAFETY: `self.ptr` is valid and registered with the input core.
+ unsafe {
+ bindings::input_report_key(self.ptr.as_ptr(), code, if value { 1 } else { 0 });
+ }
+ }
+
+ /// Reports an absolute axis position value.
+ #[inline]
+ pub fn report_abs(&self, code: u32, value: i32) {
+ // SAFETY: `self.ptr` is valid and registered with the input core.
+ unsafe {
+ bindings::input_report_abs(self.ptr.as_ptr(), code, value);
+ }
+ }
+
+ /// Flushes and synchronizes all reported events into a single input packet.
+ #[inline]
+ pub fn sync(&self) {
+ // SAFETY: `self.ptr` is valid and registered with the input core.
+ unsafe {
+ bindings::input_sync(self.ptr.as_ptr());
+ }
+ }
+}
+
+impl Drop for RegisteredDevice {
+ fn drop(&mut self) {
+ // SAFETY: `self.ptr` was successfully registered with `input_register_device`.
+ // Unregistering automatically unbinds and frees the device.
+ unsafe {
+ bindings::input_unregister_device(self.ptr.as_ptr());
+ }
+ }
+}
+
+// SAFETY: `RegisteredDevice` can be safely transferred across threads.
+unsafe impl Send for RegisteredDevice {}
+// SAFETY: Event reporting on `input_dev` is thread-safe (protected by internal locks in the
+// input core).
+unsafe impl Sync for RegisteredDevice {}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 1ab41129d557..ff5c978758d0 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -88,6 +88,8 @@
#[doc(hidden)]
pub mod impl_flags;
pub mod init;
+#[cfg(CONFIG_INPUT = "y")]
+pub mod input;
pub mod interop;
pub mod interrupt;
pub mod io;
--
2.56.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] rust: serio: add serio bus abstractions and port Gravis Stinger driver
2026-11-06 20:14 [PATCH v2 0/2] rust: input: add input/serio abstractions and port Gravis Stinger driver clicedvly768
2026-11-06 20:14 ` [PATCH v2 1/2] rust: input: add input device abstractions clicedvly768
@ 2026-11-06 20:14 ` clicedvly768
2026-11-06 20:33 ` sashiko-bot
1 sibling, 1 reply; 5+ messages in thread
From: clicedvly768 @ 2026-11-06 20:14 UTC (permalink / raw)
To: ojeda, dmitry.torokhov; +Cc: rust-for-linux, linux-input, clicedvly768
Add safe Rust abstractions for the serio (Serial I/O) bus subsystem and
port the legacy Gravis Stinger serial gamepad driver to safe Rust.
This introduces:
- `DeviceId`: a transparent wrapper around `struct serio_device_id`
implementing `RawDeviceId` for type-safe matching tables.
- `IdTable` and `serio_device_table!` macro: guarantees a zero sentinel
termination entry at compile time and creates modpost device aliases.
- `Device`: a borrowed serio port device instance implementing
`AsRef<device::Device>` and `Deref<Target = device::Device>`.
- `IrqReturn`: re-exports `kernel::irq::IrqReturn` for interrupt handling.
- `Driver` trait: safe interface for serio drivers (`connect`,
`interrupt`, `disconnect`).
- `Registration<T>`: safe RAII registration adapter that stores
`Opaque<bindings::serio_driver>` inside `Pin<KBox<...>>` to avoid
aliasing violations, and calls `__serio_register_driver` with the
caller's `&'static ThisModule` to correctly track module ownership.
- Port management in `connect_callback`: opens the serio port prior
to probe completion, sets driver data safely, and correctly propagates
negative error numbers.
- `stinger_rust`: port of the Gravis Stinger serial gamepad driver in
safe Rust (`drivers/input/joystick/stinger_rust.rs`), containing zero
unsafe blocks, using `kernel::input` and `kernel::serio` abstractions.
- Non-regression Kconfig: adds `CONFIG_JOYSTICK_STINGER_RUST` guarded on
`RUST && SERIO=y && INPUT=y` while retaining the original C driver
(`stinger.c`) to maintain hardware support on non-Rust configurations
and architectures without HAVE_RUST.
- Gated module entry `#[cfg(CONFIG_SERIO = "y")]` and helper guards
`#if IS_REACHABLE(CONFIG_SERIO)` to avoid undefined symbols when serio
is disabled or modular.
Signed-off-by: clicedvly768 <harepro29@gmail.com>
---
drivers/input/joystick/Kconfig | 14 ++
drivers/input/joystick/Makefile | 4 +
drivers/input/joystick/stinger_rust.rs | 178 +++++++++++++++++
rust/bindings/bindings_helper.h | 1 +
rust/helpers/helpers.c | 1 +
rust/helpers/serio.c | 17 ++
rust/kernel/lib.rs | 2 +
rust/kernel/serio.rs | 256 +++++++++++++++++++++++++
8 files changed, 473 insertions(+)
create mode 100644 drivers/input/joystick/stinger_rust.rs
create mode 100644 rust/helpers/serio.c
create mode 100644 rust/kernel/serio.rs
diff --git a/drivers/input/joystick/Kconfig b/drivers/input/joystick/Kconfig
index 7755e5b454d2..89cc1dd9d865 100644
--- a/drivers/input/joystick/Kconfig
+++ b/drivers/input/joystick/Kconfig
@@ -197,6 +197,20 @@ config JOYSTICK_STINGER
To compile this driver as a module, choose M here: the
module will be called stinger.
+config JOYSTICK_STINGER_RUST
+ bool "Safe Rust implementation of Gravis Stinger gamepad driver"
+ depends on JOYSTICK_STINGER
+ depends on RUST && SERIO = y && INPUT = y
+ default y
+ help
+ Say Y here to use the safe Rust rewrite of the Gravis Stinger
+ gamepad driver instead of the legacy C implementation.
+
+ The Rust driver provides memory safety guarantees while preserving
+ full hardware protocol compatibility.
+
+ If unsure, say Y.
+
config JOYSTICK_TWIDJOY
tristate "Twiddler as a joystick"
select SERIO
diff --git a/drivers/input/joystick/Makefile b/drivers/input/joystick/Makefile
index 9976f596a920..786f950dcf52 100644
--- a/drivers/input/joystick/Makefile
+++ b/drivers/input/joystick/Makefile
@@ -33,7 +33,11 @@ obj-$(CONFIG_JOYSTICK_SENSEHAT) += sensehat-joystick.o
obj-$(CONFIG_JOYSTICK_SIDEWINDER) += sidewinder.o
obj-$(CONFIG_JOYSTICK_SPACEBALL) += spaceball.o
obj-$(CONFIG_JOYSTICK_SPACEORB) += spaceorb.o
+ifdef CONFIG_JOYSTICK_STINGER_RUST
+obj-$(CONFIG_JOYSTICK_STINGER) += stinger_rust.o
+else
obj-$(CONFIG_JOYSTICK_STINGER) += stinger.o
+endif
obj-$(CONFIG_JOYSTICK_TMDC) += tmdc.o
obj-$(CONFIG_JOYSTICK_TURBOGRAFX) += turbografx.o
obj-$(CONFIG_JOYSTICK_TWIDJOY) += twidjoy.o
diff --git a/drivers/input/joystick/stinger_rust.rs b/drivers/input/joystick/stinger_rust.rs
new file mode 100644
index 000000000000..6250d628ee3d
--- /dev/null
+++ b/drivers/input/joystick/stinger_rust.rs
@@ -0,0 +1,178 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+//! Gravis Stinger gamepad driver written in safe Rust.
+//!
+//! Rust port of legacy C driver `drivers/input/joystick/stinger.c`.
+//!
+//! Authors:
+//! - Original C driver: Vojtech Pavlik <vojtech@ucw.cz>, Mark Fletcher
+//! - Rust port: Rust for Linux Contributors
+
+use kernel::{
+ bindings,
+ input::{bus, event_type, AllocatedDevice, RegisteredDevice},
+ prelude::*,
+ serio,
+ types::ForeignOwnable,
+};
+
+const DRIVER_NAME: &CStr = c"stinger";
+const DRIVER_DESC: &CStr = c"Gravis Stinger gamepad";
+const DRIVER_PHYS: &CStr = c"serio0/input0";
+
+const STINGER_MAX_LENGTH: usize = 8;
+
+kernel::serio_device_table!(
+ STINGER_IDS,
+ [serio::DeviceId::new(
+ bindings::SERIO_RS232 as u8,
+ bindings::SERIO_ANY as u8,
+ bindings::SERIO_ANY as u8,
+ bindings::SERIO_STINGER as u8,
+ )]
+);
+
+/// Per-device state for an attached Gravis Stinger gamepad.
+struct Stinger {
+ input: RegisteredDevice,
+ idx: usize,
+ data: [u8; STINGER_MAX_LENGTH],
+}
+
+impl Stinger {
+ /// Decodes incoming 4-byte packets from the Gravis Stinger and reports input events.
+ fn process_packet(&self) {
+ if self.idx == 0 {
+ return;
+ }
+
+ let d0 = i32::from(self.data[0]);
+ let d1 = i32::from(self.data[1]);
+ let d2 = i32::from(self.data[2]);
+ let d3 = i32::from(self.data[3]);
+
+ // Report button states (active high bits in packet)
+ self.input
+ .report_key(bindings::BTN_A, ((d0 & 0x20) >> 5) != 0);
+ self.input
+ .report_key(bindings::BTN_B, ((d0 & 0x10) >> 4) != 0);
+ self.input
+ .report_key(bindings::BTN_C, ((d0 & 0x08) >> 3) != 0);
+ self.input
+ .report_key(bindings::BTN_X, ((d0 & 0x04) >> 2) != 0);
+ self.input
+ .report_key(bindings::BTN_Y, ((d3 & 0x20) >> 5) != 0);
+ self.input
+ .report_key(bindings::BTN_Z, ((d3 & 0x10) >> 4) != 0);
+ self.input
+ .report_key(bindings::BTN_TL, ((d3 & 0x08) >> 3) != 0);
+ self.input
+ .report_key(bindings::BTN_TR, ((d3 & 0x04) >> 2) != 0);
+ self.input
+ .report_key(bindings::BTN_SELECT, ((d3 & 0x02) >> 1) != 0);
+ self.input.report_key(bindings::BTN_START, (d3 & 0x01) != 0);
+
+ // Report 2-axis position: coordinates in range -64..64
+ self.input
+ .report_abs(bindings::ABS_X, (d1 & 0x3f) - ((d0 & 0x01) << 6));
+ self.input
+ .report_abs(bindings::ABS_Y, ((d0 & 0x02) << 5) - (d2 & 0x3f));
+
+ self.input.sync();
+ }
+}
+
+/// The safe serio driver implementation.
+struct StingerDriver;
+
+impl serio::Driver for StingerDriver {
+ type Data = KBox<Stinger>;
+
+ const ID_TABLE: serio::IdTable = &STINGER_IDS;
+
+ fn connect(dev: &mut serio::Device) -> Result<Self::Data> {
+ let mut input_alloc = AllocatedDevice::new()?;
+
+ input_alloc.set_name(DRIVER_DESC);
+ input_alloc.set_phys(DRIVER_PHYS);
+ input_alloc.set_id(bus::RS232, bindings::SERIO_STINGER as u16, 0x0001, 0x0100);
+ input_alloc.set_parent(dev);
+
+ // Configure supported buttons
+ for btn in [
+ bindings::BTN_A,
+ bindings::BTN_B,
+ bindings::BTN_C,
+ bindings::BTN_X,
+ bindings::BTN_Y,
+ bindings::BTN_Z,
+ bindings::BTN_TL,
+ bindings::BTN_TR,
+ bindings::BTN_START,
+ bindings::BTN_SELECT,
+ ] {
+ input_alloc.set_capability(event_type::KEY, btn);
+ }
+
+ // Configure 2-axis joystick with flat margin of 4
+ input_alloc.set_capability(event_type::ABS, bindings::ABS_X);
+ input_alloc.set_capability(event_type::ABS, bindings::ABS_Y);
+ input_alloc.set_abs_params(bindings::ABS_X, -64, 64, 0, 4)?;
+ input_alloc.set_abs_params(bindings::ABS_Y, -64, 64, 0, 4)?;
+
+ let registered = input_alloc.register()?;
+
+ let stinger = KBox::new(
+ Stinger {
+ input: registered,
+ idx: 0,
+ data: [0; STINGER_MAX_LENGTH],
+ },
+ GFP_KERNEL,
+ )?;
+
+ Ok(stinger)
+ }
+
+ fn interrupt(
+ stinger: &mut <Self::Data as ForeignOwnable>::BorrowedMut<'_>,
+ data: u8,
+ _flags: u32,
+ ) -> serio::IrqReturn {
+ if stinger.idx < STINGER_MAX_LENGTH {
+ stinger.data[stinger.idx] = data;
+ stinger.idx += 1;
+ }
+
+ if stinger.idx == 4 {
+ stinger.process_packet();
+ stinger.idx = 0;
+ }
+
+ serio::IrqReturn::Handled
+ }
+
+ fn disconnect(_data: Self::Data) {
+ // Automatically drops `stinger`, unregistering and freeing the input device.
+ }
+}
+
+module! {
+ type: StingerModule,
+ name: "stinger_rust",
+ authors: ["Rust for Linux Contributors"],
+ description: "Gravis Stinger gamepad driver in safe Rust",
+ license: "GPL",
+}
+
+struct StingerModule {
+ _reg: serio::Registration<StingerDriver>,
+}
+
+impl kernel::Module for StingerModule {
+ fn init(module: &'static ThisModule) -> Result<Self> {
+ let reg = serio::Registration::<StingerDriver>::new(DRIVER_NAME, module)?;
+ pr_info!("Gravis Stinger safe Rust driver initialized\n");
+ Ok(Self { _reg: reg })
+ }
+}
diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 4b09d3980e10..0e44af5d770c 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -89,6 +89,7 @@
#include <linux/sched.h>
#include <linux/security.h>
#include <linux/serdev.h>
+#include <linux/serio.h>
#include <linux/slab.h>
#include <linux/sys_soc.h>
#include <linux/task_work.h>
diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
index 77cec85ad153..61cf3f77e85a 100644
--- a/rust/helpers/helpers.c
+++ b/rust/helpers/helpers.c
@@ -96,6 +96,7 @@
#include "scatterlist.c"
#include "security.c"
#include "serdev.c"
+#include "serio.c"
#include "signal.c"
#include "slab.c"
#include "spinlock.c"
diff --git a/rust/helpers/serio.c b/rust/helpers/serio.c
new file mode 100644
index 000000000000..849d0d48ac62
--- /dev/null
+++ b/rust/helpers/serio.c
@@ -0,0 +1,17 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/serio.h>
+
+#if IS_REACHABLE(CONFIG_SERIO)
+
+__rust_helper void *rust_helper_serio_get_drvdata(struct serio *serio)
+{
+ return serio_get_drvdata(serio);
+}
+
+__rust_helper void rust_helper_serio_set_drvdata(struct serio *serio, void *data)
+{
+ serio_set_drvdata(serio, data);
+}
+
+#endif /* IS_REACHABLE(CONFIG_SERIO) */
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index ff5c978758d0..711b7ef06b57 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -134,6 +134,8 @@
pub mod seq_file;
#[cfg(CONFIG_RUST_SERIAL_DEV_BUS_ABSTRACTIONS)]
pub mod serdev;
+#[cfg(CONFIG_SERIO = "y")]
+pub mod serio;
pub mod sizes;
#[cfg(CONFIG_SOC_BUS)]
pub mod soc;
diff --git a/rust/kernel/serio.rs b/rust/kernel/serio.rs
new file mode 100644
index 000000000000..b4a8ec7139a2
--- /dev/null
+++ b/rust/kernel/serio.rs
@@ -0,0 +1,256 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Serio (Serial I/O) bus abstractions for safe Rust drivers.
+//!
+//! Provides a safe Rust interface for serio devices and drivers in the Linux kernel,
+//! supporting input devices such as serial gamepads (e.g. Gravis Stinger), mice, and keyboards.
+
+use crate::{
+ bindings, device,
+ device_id::{IdTable as GenericIdTable, RawDeviceId},
+ error::{to_result, Result},
+ ffi::{c_int, c_uchar, c_uint},
+ prelude::*,
+ types::{ForeignOwnable, Opaque},
+ ThisModule,
+};
+use core::{marker::PhantomData, ops::Deref, pin::Pin, ptr::NonNull};
+
+pub use crate::irq::IrqReturn;
+
+/// Identifies supported serio hardware devices for auto-probing and matching.
+#[repr(transparent)]
+#[derive(Copy, Clone)]
+pub struct DeviceId(bindings::serio_device_id);
+
+impl DeviceId {
+ /// Creates a new serio device identifier.
+ #[inline]
+ pub const fn new(type_: u8, extra: u8, id: u8, proto: u8) -> Self {
+ Self(bindings::serio_device_id {
+ type_,
+ extra,
+ id,
+ proto,
+ })
+ }
+}
+
+// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper around `bindings::serio_device_id`.
+unsafe impl RawDeviceId for DeviceId {
+ type RawType = bindings::serio_device_id;
+}
+
+/// Type alias for serio device ID tables.
+pub type IdTable = &'static dyn GenericIdTable<DeviceId, ()>;
+
+/// Creates a serio device table with a zero sentinel for module autoloading and matching.
+#[macro_export]
+macro_rules! serio_device_table {
+ ($table_name:ident, [$($id:expr),* $(,)?]) => {
+ $crate::module_device_table!(
+ "serio",
+ $crate::serio::DeviceId,
+ $table_name,
+ @none,
+ [$($id),*]
+ );
+ };
+}
+
+/// A serio port device instance passed to driver callbacks.
+pub struct Device {
+ ptr: NonNull<bindings::serio>,
+}
+
+impl Device {
+ /// Returns the raw pointer to the underlying `struct serio`.
+ #[inline]
+ pub fn as_raw(&self) -> *mut bindings::serio {
+ self.ptr.as_ptr()
+ }
+
+ /// Reads the physical device path into a buffer.
+ #[inline]
+ pub fn phys(&self) -> &[u8; 32] {
+ // SAFETY: `self.ptr` points to a valid `struct serio`.
+ unsafe { &(*self.ptr.as_ptr()).phys }
+ }
+}
+
+impl AsRef<device::Device> for Device {
+ #[inline]
+ fn as_ref(&self) -> &device::Device {
+ // SAFETY: `self.ptr` points to a valid `struct serio`, which embeds `struct device dev`.
+ unsafe {
+ let dev = core::ptr::addr_of_mut!((*self.ptr.as_ptr()).dev);
+ device::Device::from_raw(dev)
+ }
+ }
+}
+
+impl Deref for Device {
+ type Target = device::Device;
+
+ #[inline]
+ fn deref(&self) -> &Self::Target {
+ self.as_ref()
+ }
+}
+
+/// Trait implemented by serio device drivers.
+pub trait Driver: Send + Sync + Sized {
+ /// Context data stored per connected device instance.
+ type Data: ForeignOwnable + Send + Sync;
+
+ /// Device ID match table.
+ const ID_TABLE: IdTable;
+
+ /// Called when a matching serio port is detected.
+ fn connect(dev: &mut Device) -> Result<Self::Data>;
+
+ /// Called in interrupt context when a new byte arrives from the serio port.
+ fn interrupt(
+ data: &mut <Self::Data as ForeignOwnable>::BorrowedMut<'_>,
+ byte: u8,
+ flags: u32,
+ ) -> IrqReturn;
+
+ /// Called when the device is disconnected.
+ fn disconnect(_data: Self::Data) {}
+}
+
+/// Driver adapter that bridges the safe Rust [`Driver`] trait to the C serio bus subsystem.
+pub struct Registration<T: Driver> {
+ drv: Pin<KBox<Opaque<bindings::serio_driver>>>,
+ _p: PhantomData<T>,
+}
+
+impl<T: Driver> Registration<T> {
+ /// Registers a serio driver with the kernel.
+ pub fn new(name: &'static CStr, module: &'static ThisModule) -> Result<Self> {
+ let drv = bindings::serio_driver {
+ description: name.as_char_ptr(),
+ id_table: T::ID_TABLE.as_ptr(),
+ manual_bind: false,
+ write_wakeup: None,
+ interrupt: Some(Self::interrupt_callback),
+ connect: Some(Self::connect_callback),
+ reconnect: None,
+ fast_reconnect: None,
+ disconnect: Some(Self::disconnect_callback),
+ cleanup: None,
+ driver: bindings::device_driver {
+ name: name.as_char_ptr(),
+ ..Default::default()
+ },
+ };
+
+ let drv_box = KBox::into_pin(KBox::new(Opaque::new(drv), GFP_KERNEL)?);
+
+ // SAFETY: `drv_box` is pinned on the kernel heap at a permanent address.
+ // `module` and `name` are valid static references.
+ let ret = unsafe {
+ bindings::__serio_register_driver(drv_box.get(), module.as_ptr(), name.as_char_ptr())
+ };
+ to_result(ret)?;
+
+ Ok(Self {
+ drv: drv_box,
+ _p: PhantomData,
+ })
+ }
+
+ /// # Safety
+ ///
+ /// Must only be invoked by the serio subsystem when a matching port is probed.
+ /// `serio` and `drv` must be valid non-null pointers provided by the kernel.
+ unsafe extern "C" fn connect_callback(
+ serio: *mut bindings::serio,
+ drv: *mut bindings::serio_driver,
+ ) -> c_int {
+ let Some(serio_nn) = NonNull::new(serio) else {
+ return crate::error::code::EINVAL.to_errno();
+ };
+
+ // SAFETY: `serio` and `drv` are valid non-null pointers provided by the kernel.
+ let ret = unsafe { bindings::serio_open(serio, drv) };
+ if ret != 0 {
+ return ret;
+ }
+
+ let mut dev = Device { ptr: serio_nn };
+
+ let data = match T::connect(&mut dev) {
+ Ok(d) => d,
+ Err(e) => {
+ // SAFETY: Close the opened port on probe error.
+ unsafe { bindings::serio_close(serio) };
+ return e.to_errno();
+ }
+ };
+
+ let raw_data = data.into_foreign();
+ // SAFETY: `serio` is valid; store driver data pointer.
+ unsafe {
+ bindings::serio_set_drvdata(serio, raw_data.cast());
+ }
+
+ 0
+ }
+
+ /// # Safety
+ ///
+ /// Must only be invoked by the serio subsystem during an interrupt on a connected port.
+ /// `serio` must be a valid pointer.
+ unsafe extern "C" fn interrupt_callback(
+ serio: *mut bindings::serio,
+ data: c_uchar,
+ flags: c_uint,
+ ) -> bindings::irqreturn_t {
+ // SAFETY: `serio` is valid.
+ let raw = unsafe { bindings::serio_get_drvdata(serio) };
+ if raw.is_null() {
+ return bindings::irqreturn::IRQ_NONE;
+ }
+
+ // SAFETY: `raw` was created via `into_foreign()` in `connect_callback`.
+ let mut borrowed = unsafe { <T::Data as ForeignOwnable>::borrow_mut(raw) };
+ let ret = T::interrupt(&mut borrowed, data, flags);
+ bindings::irqreturn_t(ret as c_uint)
+ }
+
+ /// # Safety
+ ///
+ /// Must only be invoked by the serio subsystem when disconnecting a port.
+ /// `serio` must be a valid pointer.
+ unsafe extern "C" fn disconnect_callback(serio: *mut bindings::serio) {
+ // SAFETY: `serio` is valid.
+ let raw = unsafe { bindings::serio_get_drvdata(serio) };
+ if raw.is_null() {
+ return;
+ }
+
+ // SAFETY: Close port, clear drvdata, and pass owned data back to disconnect.
+ unsafe {
+ bindings::serio_close(serio);
+ bindings::serio_set_drvdata(serio, core::ptr::null_mut());
+ let data = <T::Data as ForeignOwnable>::from_foreign(raw);
+ T::disconnect(data);
+ }
+ }
+}
+
+impl<T: Driver> Drop for Registration<T> {
+ fn drop(&mut self) {
+ // SAFETY: Unregister driver with kernel. `self.drv` is valid and registered.
+ unsafe {
+ bindings::serio_unregister_driver(self.drv.get());
+ }
+ }
+}
+
+// SAFETY: `Registration` can be safely sent to another thread.
+unsafe impl<T: Driver> Send for Registration<T> {}
+// SAFETY: `Registration` can be safely shared across threads.
+unsafe impl<T: Driver> Sync for Registration<T> {}
--
2.56.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] rust: input: add input device abstractions
2026-11-06 20:14 ` [PATCH v2 1/2] rust: input: add input device abstractions clicedvly768
@ 2026-11-06 20:27 ` sashiko-bot
0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-11-06 20:27 UTC (permalink / raw)
To: clicedvly768; +Cc: linux-input, dmitry.torokhov, ojeda, gary
Thank you for your contribution! Sashiko AI review found 4 potential
issues to consider:
- [Medium] rust: input: unreferenced parent pointer in
AllocatedDevice::set_parent()
- [Low] rust: input: missing #[inline] on C wrapper methods in input.rs
- [Low] rust: input: non-vertical import style and redundant imports in
input.rs
- [Low] rust: input: duplicate error cleanup in
AllocatedDevice::register()
--
commit e772f543b269b5d6668847be715161384d0d5953
Author: clicedvly768 <harepro29@gmail.com>
rust: input: add input device abstractions
This commit adds Rust abstractions for allocating, configuring, and
registering Linux input devices (struct input_dev).
> diff --git a/rust/kernel/input.rs b/rust/kernel/input.rs
> new file mode 100644
> index 0000000000000..bef0c3a07da06
> --- /dev/null
> +++ b/rust/kernel/input.rs
> @@ -0,0 +1,214 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Input subsystem abstractions for safe Rust drivers.
> +//!
> +//! Provides a safe Rust object-oriented interface around Linux `struct input_dev`,
> +//! ensuring safe allocation, capability configuration, registration, and RAII cleanup.
> +
> +use crate::{
> + bindings, device,
> + error::{
> + code::{EINVAL, ENOMEM},
> + Result,
> + },
> + prelude::*,
> + str::CStr,
> +};
[Severity: Low]
This isn't a bug, but the imports here group multiple items on a single line
without trailing comments instead of using the kernel vertical import style.
Also, EINVAL, ENOMEM, Error, Result, and CStr are already exported by
crate::prelude::*, so importing them explicitly is redundant.
[ ... ]
> + /// Sets the parent device for this input device.
> + #[inline]
> + pub fn set_parent(&mut self, parent: &device::Device) {
> + // SAFETY: `self.ptr` is valid, and `parent.as_raw()` returns a valid `struct device`.
> + unsafe {
> + (*self.ptr.as_ptr()).dev.parent = parent.as_raw();
> + }
> + }
[Severity: Medium]
Can AllocatedDevice::set_parent() leave dev.parent as a dangling pointer?
set_parent() takes a short-lived &device::Device borrow and stores
parent.as_raw() in dev.parent without holding a reference count or tying the
lifetime of AllocatedDevice to parent. The C driver core does not call
get_device(dev->parent) until input_register_device() -> device_add(), and
input_alloc_absinfo() also dereferences dev->dev.parent before registration on
allocation failure. If a caller drops the parent device after set_parent()
returns, a subsequent call to set_capability(), set_abs_params(), or
register() from safe Rust will trigger a use-after-free.
> + /// Enables support for a capability (e.g. key or axis).
> + #[inline]
> + pub fn set_capability(&mut self, type_: u32, code: u32) {
> + // SAFETY: `self.ptr` is valid and type/code are valid capability values.
> + unsafe {
> + bindings::input_set_capability(self.ptr.as_ptr(), type_, code);
> + }
> + }
> +
> + /// Sets parameter limits for an absolute axis.
> + ///
> + /// Bounds-checks `axis < ABS_CNT` (0x40) to prevent out-of-bounds writes
> + /// to the `absinfo` array in `struct input_dev`.
> + pub fn set_abs_params(
> + &mut self,
> + axis: u32,
> + min: i32,
> + max: i32,
> + fuzz: i32,
> + flat: i32,
> + ) -> Result {
> + if axis >= bindings::ABS_CNT {
> + return Err(EINVAL);
> + }
> +
> + // SAFETY: `self.ptr` is valid and `axis` has been verified within bounds.
> + unsafe {
> + bindings::input_set_abs_params(self.ptr.as_ptr(), axis, min, max, fuzz, flat);
> + }
> + Ok(())
> + }
> +
> + /// Registers the input device with the kernel input core.
> + ///
> + /// Consumes the `AllocatedDevice` and returns a [`RegisteredDevice`].
> + pub fn register(self) -> Result<RegisteredDevice> {
> + let ptr = self.ptr;
> + // Don't run the AllocatedDevice drop destructor, registration takes ownership.
> + core::mem::forget(self);
> +
> + // SAFETY: `ptr` is valid and was allocated by `input_allocate_device`.
> + let ret = unsafe { bindings::input_register_device(ptr.as_ptr()) };
> + if ret != 0 {
> + // SAFETY: Free the allocated device if registration failed.
> + unsafe {
> + bindings::input_free_device(ptr.as_ptr());
> + }
> + return Err(crate::error::Error::from_errno(ret));
> + }
> +
> + Ok(RegisteredDevice { ptr })
> + }
> +}
[Severity: Low]
This isn't a bug, but calling core::mem::forget(self) before
input_register_device() forces the error path in register() to manually call
input_free_device() and Error::from_errno(), duplicating Drop::drop() for
AllocatedDevice and error::to_result(). Checking
to_result(unsafe { bindings::input_register_device(self.ptr.as_ptr()) })?
before forgetting self would let Drop handle cleanup on failure.
> +impl Drop for AllocatedDevice {
> + fn drop(&mut self) {
> + // SAFETY: `self.ptr` was allocated by `input_allocate_device` and has not been registered.
> + unsafe {
> + bindings::input_free_device(self.ptr.as_ptr());
> + }
> + }
> +}
[ ... ]
> +impl Drop for RegisteredDevice {
> + fn drop(&mut self) {
> + // SAFETY: `self.ptr` was successfully registered with `input_register_device`.
> + // Unregistering automatically unbinds and frees the device.
> + unsafe {
> + bindings::input_unregister_device(self.ptr.as_ptr());
> + }
> + }
> +}
[Severity: Low]
Small functions in kernel crate abstractions that forward to C bindings should
be marked #[inline]. AllocatedDevice::new(),
AllocatedDevice::set_abs_params(), AllocatedDevice::register(),
<AllocatedDevice as Drop>::drop(), and <RegisteredDevice as Drop>::drop() are
missing #[inline].
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261106201429.376997-1-harepro29@gmail.com?part=1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] rust: serio: add serio bus abstractions and port Gravis Stinger driver
2026-11-06 20:14 ` [PATCH v2 2/2] rust: serio: add serio bus abstractions and port Gravis Stinger driver clicedvly768
@ 2026-11-06 20:33 ` sashiko-bot
0 siblings, 0 replies; 5+ messages in thread
From: sashiko-bot @ 2026-11-06 20:33 UTC (permalink / raw)
To: clicedvly768; +Cc: linux-input, dmitry.torokhov, ojeda, gary
Thank you for your contribution! Sashiko AI review found 5 potential
issues to consider:
- [High] rust: serio: premature serio_open() and unlocked drvdata in
connect_callback()
- [Medium] input: stinger_rust changes device name, phys path,
description, and module name
- [Medium] rust: serio: invalid irqreturn_t and IRQ_NONE bindings in
interrupt_callback()
- [Low] rust: serio: Device and Registration bypass generic driver
abstractions
- [Low] rust: serio: non-vertical import style in serio.rs and
stinger_rust.rs
--
commit 986d37559af3527a1a01463ffbfccd5c47d94c58
Author: clicedvly768 <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/drivers/input/joystick/stinger_rust.rs b/drivers/input/joystick/stinger_rust.rs
> new file mode 100644
> index 0000000000000..6250d628ee3da
> --- /dev/null
> +++ b/drivers/input/joystick/stinger_rust.rs
[ ... ]
> +const DRIVER_NAME: &CStr = c"stinger";
> +const DRIVER_DESC: &CStr = c"Gravis Stinger gamepad";
> +const DRIVER_PHYS: &CStr = c"serio0/input0";
[ ... ]
> + fn connect(dev: &mut serio::Device) -> Result<Self::Data> {
> + let mut input_alloc = AllocatedDevice::new()?;
> +
> + input_alloc.set_name(DRIVER_DESC);
> + input_alloc.set_phys(DRIVER_PHYS);
[Severity: Medium]
Does replacing stinger.c with stinger_rust.rs when
CONFIG_JOYSTICK_STINGER_RUST=y change the userspace-visible device and
module attributes?
In drivers/input/joystick/stinger.c, stinger_connect() sets input_dev->name
to "Gravis Stinger" and formats input_dev->phys dynamically as "%s/input0"
from serio->phys. Setting the name to "Gravis Stinger gamepad" breaks
userspace controller mappings and udev rules that match EVIOCGNAME, and
hardcoding "serio0/input0" (leaving serio::Device::phys() unused) reports
the wrong physical path on any port other than serio0 or when multiple
devices are attached.
In addition, drivers/input/joystick/Makefile builds stinger_rust.ko (with
module! name "stinger_rust") instead of stinger.ko as documented in the
CONFIG_JOYSTICK_STINGER Kconfig help text, and Registration::new() sets
serio_driver.description to "stinger" instead of "Gravis Stinger gamepad
driver".
> diff --git a/rust/kernel/serio.rs b/rust/kernel/serio.rs
> new file mode 100644
> index 0000000000000..b4a8ec7139a28
> --- /dev/null
> +++ b/rust/kernel/serio.rs
> @@ -0,0 +1,256 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Serio (Serial I/O) bus abstractions for safe Rust drivers.
> +//!
> +//! Provides a safe Rust interface for serio devices and drivers in the Linux kernel,
> +//! supporting input devices such as serial gamepads (e.g. Gravis Stinger), mice, and keyboards.
> +
> +use crate::{
> + bindings, device,
> + device_id::{IdTable as GenericIdTable, RawDeviceId},
> + error::{to_result, Result},
> + ffi::{c_int, c_uchar, c_uint},
> + prelude::*,
> + types::{ForeignOwnable, Opaque},
> + ThisModule,
> +};
> +use core::{marker::PhantomData, ops::Deref, pin::Pin, ptr::NonNull};
[Severity: Low]
This isn't a bug, but the imports here and in
drivers/input/joystick/stinger_rust.rs group multiple items on single
horizontal lines instead of following the kernel vertical import style in
Documentation/rust/coding-guidelines.rst.
[ ... ]
> +/// A serio port device instance passed to driver callbacks.
> +pub struct Device {
> + ptr: NonNull<bindings::serio>,
> +}
[ ... ]
> +/// Driver adapter that bridges the safe Rust [`Driver`] trait to the C serio bus subsystem.
> +pub struct Registration<T: Driver> {
> + drv: Pin<KBox<Opaque<bindings::serio_driver>>>,
> + _p: PhantomData<T>,
> +}
[Severity: Low]
Should serio::Device and serio::Registration use the generic bus device and
driver registration infrastructure in the kernel crate instead of custom
wrappers?
Other bus abstractions (such as serdev, i2c, platform, and pci) define
Device<Ctx: device::DeviceContext = device::Normal> as a repr(transparent)
wrapper around Opaque<bindings::...> implementing device::AsBusDevice<Ctx>
and AlwaysRefCounted, and implement driver::DriverLayout and
driver::RegistrationOps on an Adapter<T> type. Because serio::Device lacks
a DeviceContext parameter, driver callbacks do not receive
&serio::Device<device::Bound> and cannot use device-managed resources such
as devres::register() or Devres::new().
[ ... ]
> + unsafe extern "C" fn connect_callback(
> + serio: *mut bindings::serio,
> + drv: *mut bindings::serio_driver,
> + ) -> c_int {
> + let Some(serio_nn) = NonNull::new(serio) else {
> + return crate::error::code::EINVAL.to_errno();
> + };
> +
> + // SAFETY: `serio` and `drv` are valid non-null pointers provided by the kernel.
> + let ret = unsafe { bindings::serio_open(serio, drv) };
> + if ret != 0 {
> + return ret;
> + }
> +
> + let mut dev = Device { ptr: serio_nn };
> +
> + let data = match T::connect(&mut dev) {
> + Ok(d) => d,
> + Err(e) => {
> + // SAFETY: Close the opened port on probe error.
> + unsafe { bindings::serio_close(serio) };
> + return e.to_errno();
> + }
> + };
> +
> + let raw_data = data.into_foreign();
> + // SAFETY: `serio` is valid; store driver data pointer.
> + unsafe {
> + bindings::serio_set_drvdata(serio, raw_data.cast());
> + }
> +
> + 0
> + }
[Severity: High]
Does calling serio_open() before T::connect() and serio_set_drvdata() race
with incoming serial interrupts?
serio_open() binds serio->drv under serio->lock and immediately enables
port interrupts via serio->open(serio), while T::connect() sleeps (for
example in AllocatedDevice::new(), input_alloc.register(), and KBox::new()
in StingerDriver::connect()) before drvdata is populated:
CPU0 CPU1 (<hardirq>)
connect_callback()
serio_open(serio, drv)
T::connect(&mut dev)
serio_interrupt()
spin_lock_irqsave(&serio->lock)
interrupt_callback()
serio_get_drvdata() -> NULL
return IRQ_NONE <-- drops byte
serio_set_drvdata(serio, raw_data)
interrupt_callback()
serio_get_drvdata() <-- data race
ForeignOwnable::borrow_mut(raw)
First, any bytes arriving while T::connect() runs are dropped because
drvdata is still NULL. In StingerDriver::interrupt(), packets are framed
solely by counting 4 bytes (stinger.idx == 4), so dropping 1 to 3 bytes
during connect permanently misaligns all subsequent packet decoding.
Second, serio_set_drvdata() performs a plain unlocked store to
serio->dev.driver_data without holding serio->lock or issuing a release
barrier, racing with interrupt_callback() reading serio_get_drvdata() and
dereferencing T::Data on weakly-ordered architectures.
Third, StingerDriver::connect() calls input_alloc.register() before
KBox::new(Stinger { ... }, GFP_KERNEL) and before drvdata is set, exposing
the input device to userspace before driver initialization can no longer
fail.
In C serio drivers such as stinger_connect() in
drivers/input/joystick/stinger.c, driver state is allocated and stored via
serio_set_drvdata() before calling serio_open() (which synchronizes under
serio->lock in serio_set_drv()), and input_register_device() is called
after serio_open() succeeds.
> +
> + /// # Safety
> + ///
> + /// Must only be invoked by the serio subsystem during an interrupt on a connected port.
> + /// `serio` must be a valid pointer.
> + unsafe extern "C" fn interrupt_callback(
> + serio: *mut bindings::serio,
> + data: c_uchar,
> + flags: c_uint,
> + ) -> bindings::irqreturn_t {
> + // SAFETY: `serio` is valid.
> + let raw = unsafe { bindings::serio_get_drvdata(serio) };
> + if raw.is_null() {
> + return bindings::irqreturn::IRQ_NONE;
> + }
> +
> + // SAFETY: `raw` was created via `into_foreign()` in `connect_callback`.
> + let mut borrowed = unsafe { <T::Data as ForeignOwnable>::borrow_mut(raw) };
> + let ret = T::interrupt(&mut borrowed, data, flags);
> + bindings::irqreturn_t(ret as c_uint)
> + }
[Severity: Medium]
Does interrupt_callback() fail to compile when CONFIG_SERIO=y and
CONFIG_RUST=y?
bindgen generates irqreturn and irqreturn_t as type aliases for ffi::c_uint
along with top-level constants (as seen in rust/kernel/irq/request.rs), not
a module or newtype struct. bindings::irqreturn::IRQ_NONE does not exist
(it is bindings::irqreturn_IRQ_NONE), and bindings::irqreturn_t(ret as
c_uint) attempts to call a type alias as a tuple struct constructor instead
of casting with ret as bindings::irqreturn_t.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261106201429.376997-1-harepro29@gmail.com?part=2
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-11-06 20:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-06 20:14 [PATCH v2 0/2] rust: input: add input/serio abstractions and port Gravis Stinger driver clicedvly768
2026-11-06 20:14 ` [PATCH v2 1/2] rust: input: add input device abstractions clicedvly768
2026-11-06 20:27 ` sashiko-bot
2026-11-06 20:14 ` [PATCH v2 2/2] rust: serio: add serio bus abstractions and port Gravis Stinger driver clicedvly768
2026-11-06 20:33 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox