linux-kernel - Re: [PATCH v3 0/5] rust: miscdevice: Provide sample driver using the new MiscDevice 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: <CAH5fLgj6rqVbGHrU4008fvO60fJdRWoE2SvW7nc9njPUFuzJ_A@mail.gmail.com>
Date: Fri, 6 Dec 2024 09:31:28 +0100
From: Alice Ryhl <aliceryhl@...gle.com>
To: Greg KH <gregkh@...uxfoundation.org>
Cc: Lee Jones <lee@...nel.org>, linux-kernel@...r.kernel.org, arnd@...db.de, 
	ojeda@...nel.org, alex.gaynor@...il.com, boqun.feng@...il.com, 
	gary@...yguo.net, bjorn3_gh@...tonmail.com, benno.lossin@...ton.me, 
	a.hindborg@...nel.org, tmgross@...ch.edu, rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v3 0/5] rust: miscdevice: Provide sample driver using the
 new MiscDevice bindings

On Fri, Dec 6, 2024 at 9:11 AM Greg KH <gregkh@...uxfoundation.org> wrote:
>
> On Fri, Dec 06, 2024 at 07:44:43AM +0000, Lee Jones wrote:
> > On Fri, 06 Dec 2024, Greg KH wrote:
> >
> > > On Thu, Dec 05, 2024 at 04:25:17PM +0000, Lee Jones wrote:
> > > > It has been suggested that the driver should use dev_info() instead of
> > > > pr_info() however there is currently no scaffolding to successfully pull
> > > > a 'struct device' out from driver data post register().  This is being
> > > > worked on and we will convert this over in due course.
> > >
> > > But the miscdevice.rs change provides this to you, right?  Or if not,
> > > why not?
> >
> > This does allow us to pull the 'struct device *` out from `struct
> > miscdevice`; however, since this resides in MiscDeviceRegistration,
> > which we lose access to after .init, we have no means to call it.
> >
> > Alice is going to work on a way to use ThisModule to get the
> > MiscDeviceRegistration reference back from anywhere in the module. Until
> > that piece lands, we can't call MiscDeviceRegistration::device() outside
> > of RustMiscDeviceModule.
>
> That seems crazy, as ThisModule shouldn't be dealing with a static misc
> device, what happens for dynamically created ones like all
> normal/sane/non-example drivers do?  This should "just" be a dynamic
> object that is NOT tied to the module object, or worst case, a "static"
> structure that is tied to the module I guess?
>
> Anyway, I'll let you all work it out, good luck!

If you store it somewhere else, you're probably okay. The current
place is just hard to access.

The problem is that the Rust module abstractions generate a global
variable that holds an RustMiscDeviceModule which is initialized in
init_module() and destroyed in cleanup_module(). To have safe access
to this global, we need to ensure that you access it only between
init_module() and cleanup_module(). For loadable modules, the
try_module_get() logic seems perfect, so in Miscdevice::open we have a
file pointer, which implies that the fs infrastructure took a refcount
on fops->owner, which it can only do once init_module() is done.

Unfortunately, this doesn't translate to built-in modules since the
owner pointer is just null, and try_module_get performs no checks at
all.

Also, I'm realizing now that try_module_get() succeeds even if `state
== MODULE_STATE_COMING`. :(

So in conclusion, I don't know of any way to provide safe access to
the global RustMiscDeviceModule value.

Alice