linux-kernel - Re: [PATCH 2/3] rust: miscdevice: Add additional data to MiscDeviceRegistration

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: <2025012243-myspace-woof-1d1e@gregkh>
Date: Wed, 22 Jan 2025 10:28:40 +0100
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Christian Schrefl <chrisi.schrefl@...il.com>
Cc: 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>,
	Arnd Bergmann <arnd@...db.de>, Lee Jones <lee@...nel.org>,
	rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] rust: miscdevice: Add additional data to
 MiscDeviceRegistration

On Sun, Jan 19, 2025 at 11:11:14PM +0100, Christian Schrefl wrote:
> When using the Rust miscdevice bindings, you generally embed the
> MiscDeviceRegistration within another struct:
> 
> struct MyDriverData {
>     data: SomeOtherData,
>     misc: MiscDeviceRegistration<MyMiscFile>
> }
> 
> In the `fops->open` callback of the miscdevice, you are given a
> reference to the registration, which allows you to access its fields.
> For example, as of commit 284ae0be4dca ("rust: miscdevice: Provide
> accessor to pull out miscdevice::this_device") you can access the
> internal `struct device`. However, there is still no way to access the
> `data` field in the above example, because you only have a reference to
> the registration.

What's wrong with the driver_data pointer in the misc device structure?
Shouldn't you be in control of that as you are a misc driver owner?  Or
does the misc core handle this I can't recall at the moment, sorry.

> Using container_of is also not possible to do safely. For example, if
> the destructor of `MyDriverData` runs, then the destructor of `data`
> would run before the miscdevice is deregistered, so using container_of
> to access `data` from `fops->open` could result in a UAF. A similar
> problem can happen on initialization if `misc` is not the last field to
> be initialized.
> 
> To provide a safe way to access user-defined data stored next to the
> `struct miscdevice`, make `MiscDeviceRegistration` into a container that
> can store a user-provided piece of data. This way, `fops->open` can
> access that data via the registration, since the data is stored inside
> the registration.

"next to" feels odd, that's what a container_of is for, but be careful
as to who owns the lifecycle of the object you are trying to get to.
You can't have multiple objects with different lifecycles in the same
structure (i.e. don't mix a misc device and a platform device together).

So a real example here would be good to see, can you post your driver at
the same time so that we can see what you are doing and perhaps provide
a better way to do it?

thanks,

greg k-h