linux-kernel - Re: [PATCH v2] 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: <2025020721-squirt-jam-e7f9@gregkh>
Date: Fri, 7 Feb 2025 16:15:01 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Danilo Krummrich <dakr@...nel.org>
Cc: Lyude Paul <lyude@...hat.com>, rust-for-linux@...r.kernel.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>,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	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 v2] rust/kernel: Add faux device bindings

On Fri, Feb 07, 2025 at 01:17:45PM +0100, Danilo Krummrich wrote:
> On Fri, Feb 07, 2025 at 10:25:09AM +0100, Greg Kroah-Hartman wrote:
> > On Thu, Feb 06, 2025 at 07:40:45PM -0500, Lyude Paul wrote:
> > > +
> > > +/// The registration of a faux device.
> > > +///
> > > +/// 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.
> > > +///
> > > +/// # Invariants
> > > +///
> > > +/// `self.0` always holds a valid pointer to an initialized and registered [`struct faux_device`].
> > > +///
> > > +/// [`struct faux_device`]: srctree/include/linux/device/faux.h
> > > +#[repr(transparent)]
> > > +pub struct Registration(NonNull<bindings::faux_device>);
> > > +
> > > +impl Registration {
> > > +    /// Create and register a new faux device with the given name.
> > > +    pub fn new(name: &CStr) -> Result<Self> {
> > > +        // SAFETY:
> > > +        // - `name` is copied by this function into its own storage
> > > +        // - `faux_ops` is safe to leave NULL according to the C API
> > > +        let dev = unsafe { bindings::faux_device_create(name.as_char_ptr(), null()) };
> > 
> > I'm fine with null() here, but why wouldn't a rust binding want to allow
> > this?  What's unique here that make it this way, or is it just that the
> > current users you are thinking of don't care about it?
> 
> This is what I meant when I mentioned allowing NULL for the faux_device_ops
> allows us to simplify the Rust abstraction quite a bit.
> 
> Having probe() and remove() doesn't do a lot for us in this case in Rust other
> than needing a separate faux::Driver trait with a corresponding faux::Adapter
> implementation to handle those callbacks. It'd be an unnecessary indirection.
> 
> Do you see any advantage going through probe()?

Only reason I implemented it was that we have a real user of it in the
kernel that needed it, the regulator dummy driver.  Odds are we could
rewrite the C code to not need it, but for now let's leave it.

If over time, no one else actually needs it, I'll refactor and remove
the callbacks entirely after more of the tree is moved to the faux code.

If you all don't need the callback, wonderful, I'll not complain :)

thanks,

greg k-h