linux-kernel - Re: [PATCH v5 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: <e613d12e-92d1-4746-9012-5194fa1cbecf@gmail.com>
Date: Sat, 7 Jun 2025 22:10:56 +0200
From: Christian Schrefl <chrisi.schrefl@...il.com>
To: Miguel Ojeda <ojeda@...nel.org>, Danilo Krummrich <dakr@...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>, Andreas Hindborg <a.hindborg@...nel.org>,
 Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
 Arnd Bergmann <arnd@...db.de>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>, Lee Jones <lee@...nel.org>,
 Daniel Almeida <daniel.almeida@...labora.com>,
 Benno Lossin <lossin@...nel.org>
Cc: Gerald Wisböck <gerald.wisboeck@...ther.ink>,
 rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 2/3] rust: miscdevice: add additional data to
 `MiscDeviceRegistration`

On 07.06.25 2:07 PM, Christian Schrefl wrote:
> @@ -178,18 +214,29 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {>          // SAFETY: The open call of a file can access the private data.
>          let misc_ptr = unsafe { (*raw_file).private_data };
>  
> -        // SAFETY: This is a miscdevice, so `misc_open()` set the private data to a pointer to the
> -        // associated `struct miscdevice` before calling into this method. Furthermore,
> -        // `misc_open()` ensures that the miscdevice can't be unregistered and freed during this
> -        // call to `fops_open`.
> -        let misc = unsafe { &*misc_ptr.cast::<MiscDeviceRegistration<T>>() };
> +        // This is a miscdevice, so `misc_open()` sets the private data to a pointer to the
> +        // associated `struct miscdevice` before calling into this method.
> +        let misc_ptr = misc_ptr.cast::<Opaque<bindings::miscdevice>>();
> +
> +        // SAFETY:
> +        // * `misc_open()` ensures that the `struct miscdevice` can't be unregistered and freed
> +        //   during this call to `fops_open`.
> +        // * The `misc_ptr` always points to the `inner` field of a `MiscDeviceRegistration<T>`.
> +        // * The `MiscDeviceRegistration<T>` is valid until the `struct miscdevice` was
> +        //   unregistered.
> +        // * `MiscDeviceRegistration<T>` is `Send` since `MiscDeviceRegistration::register` has a
> +        //   `Self: Send` bound and is the only way to create a `MiscDeviceRegistration`. This
> +        //   means that a reference to it can be shared between contexts.
> +        // TODO: add `assert_sync` for `MiscDeviceRegistration<T>` and
> +        // `MiscDeviceRegistration<T>::Data`.

After trying this out it this needs `: Sync` bounds on this impl block and
the `MiscDeviceOptions::into_raw` in addition to the register function.
That's not great, but could be worse.

> +        let registration = unsafe { &*container_of!(misc_ptr, MiscDeviceRegistration<T>, inner) };
>  
>          // SAFETY:
>          // * This underlying file is valid for (much longer than) the duration of `T::open`.
>          // * There is no active fdget_pos region on the file on this thread.
>          let file = unsafe { File::from_raw_file(raw_file) };
>  
> -        let ptr = match T::open(file, misc) {
> +        let ptr = match T::open(file, registration) {
>              Ok(ptr) => ptr,
>              Err(err) => return err.to_errno(),
>          };