linux-kernel - Re: [PATCH] rust/kernel: Add faux device bindings

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: <Z6VQ0D-L4Z0pxxsa@cassiopeiae>
Date: Fri, 7 Feb 2025 01:16:16 +0100
From: Danilo Krummrich <dakr@...nel.org>
To: Lyude Paul <lyude@...hat.com>
Cc: rust-for-linux@...r.kernel.org,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Maíra Canal <mairacanal@...eup.net>,
	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>,
	Wedson Almeida Filho <wedsonaf@...il.com>,
	Mika Westerberg <mika.westerberg@...ux.intel.com>,
	Xiangfei Ding <dingxiangfei2009@...il.com>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] rust/kernel: Add faux device bindings

On Thu, Feb 06, 2025 at 06:04:03PM -0500, Lyude Paul wrote:
> On Thu, 2025-02-06 at 23:30 +0100, Danilo Krummrich wrote:
> > > +//! Abstractions for the faux bus.
> > > +//!
> > > +//! This crate provides bindings for working with faux devices in kernel modules. It should be
> > > +//! preferred for creating virtual devices over the platform API.
> > 
> > "preferred" implies a bit that platform devices are still an option for that
> > (even if not preferred). Maybe just not mention it at all. But if you want to,
> > maybe something along the lines of "faux devices are the solution for the
> > historical abuse of platform devices as virtual devices"?
> > 
> > > +//!
> > > +//! C header: [`include/linux/device/faux.h`]
> > > +use crate::{bindings, device, error::from_err_ptr, prelude::*};
> > > +use core::ptr::{addr_of_mut, null, NonNull};
> > > +
> > > +/// The faux device representation.
> > > +///
> > > +/// This type represents the registration of a [`struct faux_device`]. When an instance of this type
> > > +/// is dropped, its respective faux device will be unregistered from the system.
> > 
> > Ultimately, this will be used to be passed to C APIs, such as drm_dev_alloc(),
> > which increment the reference count of the underlying struct device.
> > 
> > Should we consider that in Rust we may have a need to take additional references
> > in the future too?
> > 
> > Maybe it would be more future proof to call this structure `Registration` and
> > leave us the option to define faux::Device for reference counting later on.
> 
> Yeah I was considering calling this Registration rather than Device, but
> mainly for the reason that a device registration (at least to me) is a unique
> resource.

What about the fact that your comment says "This type represents the
registration [...]"? :-)

> I think actually taking references to the Device should be the job
> of the kernel device core though

Everyone who stores a pointer to a reference counted thing has to take a
reference.

drm_dev_init() for instance, takes a refernece because a drm_device can outlive
the parent device' (in this case the faux device') registration.

Once we get to native Rust APIs of this kind in the future, we'd need to take
our own reference of this device.

The `Registration` structure's lifetime should represent the time in which a
device is registered in the system.

Whereas the `Device` structure's lifetime should represent the lifetime of a
single reference to the device. This is exactly what pci::Device,
platform::Device, and the base device::Device do.

For the faux device it's that faux_device_create() allocates, initializes and
registers the device at once and faux_device_destroy() unregisters the device
and drops the initial reference from faux_device_create() at once. After that,
the device is unregistered, but depending on whether there are still references
held to the device, it can still be alive.

I also suggest to have a look at `MiscDeviceRegistration` registration, which is
similar from the registration side of things.