linux-kernel - Re: [PATCH RFC 1/3] rust: core abstractions for HID drivers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z9MxI0u2yCfSzTvD@cassiopeiae>
Date: Thu, 13 Mar 2025 20:25:23 +0100
From: Danilo Krummrich <dakr@...nel.org>
To: Rahul Rameshbabu <sergeantsagara@...tonmail.com>
Cc: linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
	linux-input@...r.kernel.org, dri-devel@...ts.freedesktop.org,
	Jiri Kosina <jikos@...nel.org>,
	Benjamin Tissoires <bentiss@...nel.org>,
	Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...il.com>,
	Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
	Björn Roy Baron <bjorn3_gh@...tonmail.com>,
	Benno Lossin <benno.lossin@...ton.me>,
	Andreas Hindborg <a.hindborg@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>
Subject: Re: [PATCH RFC 1/3] rust: core abstractions for HID drivers

On Thu, Mar 13, 2025 at 04:03:35PM +0000, Rahul Rameshbabu wrote:
> These abstractions enable the development of HID drivers in Rust by binding
> with the HID core C API. They provide Rust types that map to the
> equivalents in C. In this initial draft, only hid_device and hid_device_id
> are provided direct Rust type equivalents. hid_driver is specially wrapped
> with a custom Driver type. The module_hid_driver! macro provides analogous
> functionality to its C equivalent.
> 
> Future work for these abstractions would include more bindings for common
> HID-related types, such as hid_field, hid_report_enum, and hid_report.
> Providing Rust equivalents to useful core HID functions will also be
> necessary for HID driver development in Rust.
> 
> Some concerns with this initial draft
>   - The need for a DeviceId and DeviceIdShallow type.
>     + DeviceIdShallow is used to guarantee the safety requirement for the
>       Sync trait.
>   - The create_hid_driver call in the module_hid_driver! macro does not use
>     Pin semantics for passing the ID_TABLE. I could not get Pin semantics
>     to work in a const fn. I get a feeling this might be safe but need help
>     reviewing this.

For a lot of things in this patch we have common infrastructure, please see
rust/kernel/{device.rs, driver.rs, device_id.rs}. I think you should make use of
the common infrastructure that solves the corresponding problems already.

It provides generic infrastructure for handling device IDs, a generalized
Registration type, based on InPlaceModule with a common module_driver!
implementation for busses to implement their corresponding module macro, etc.

There are two busses upstream, which are based on this infrastructure:
rust/kernel/{pci.rs, platform.rs}.

There is a patch series that improves soundness of those two bus abstractions
[1], which should be taken into consideration too. Even though your
implementation isn't prone to the addressed issue, it would be good to align
things accordingly.

There is a third bus abstraction (auxiliary) on the mailing list [2], which
already implements the mentioned improvements, which you can use as canonical
example too.

[1] https://lore.kernel.org/rust-for-linux/20250313021550.133041-1-dakr@kernel.org/
[2] https://lore.kernel.org/rust-for-linux/20250313022454.147118-1-dakr@kernel.org/

> Signed-off-by: Rahul Rameshbabu <sergeantsagara@...tonmail.com>
> ---
>  drivers/hid/Kconfig             |   8 ++
>  rust/bindings/bindings_helper.h |   1 +
>  rust/kernel/hid.rs              | 245 ++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs              |   2 +
>  4 files changed, 256 insertions(+)
>  create mode 100644 rust/kernel/hid.rs