linux-kernel - Re: [PATCH v6 3/6] rust: irq: add support for non-threaded IRQs and handlers

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: <aHSmxWeIy3L-AKIV@Mac.home>
Date: Sun, 13 Jul 2025 23:42:13 -0700
From: Boqun Feng <boqun.feng@...il.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: Benno Lossin <lossin@...nel.org>,
	Daniel Almeida <daniel.almeida@...labora.com>,
	Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...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>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Rafael J. Wysocki" <rafael@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Krzysztof Wilczy´nski <kwilczynski@...nel.org>,
	linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
	linux-pci@...r.kernel.org
Subject: Re: [PATCH v6 3/6] rust: irq: add support for non-threaded IRQs and
 handlers

On Sun, Jul 13, 2025 at 02:42:41PM +0200, Danilo Krummrich wrote:
[...]
> If we use container_of!() instead or just pass the address of Self (i.e.
> Registration) to request_irq() instead,
> 
> 	pub fn device(&self) -> &Device
> 
> is absolutely possible to add to Devres, of course.
> 

One thing to notice is that in `Devres::new()`, `inner` is initialized
before `dev`:

        try_pin_init!(&this in Self {
            // INVARIANT: `inner` is properly initialized.
            inner <- Opaque::pin_init(try_pin_init!(Inner {
                data <- Revocable::new(data),
                devm <- Completion::new(),
                revoke <- Completion::new(),
            })),
	    
For `irq::Registration`, request_irq() is called at `inner`
initialization. So now interrupts can happen at any moment, but `dev` is
still uninitialized.

            callback,
            dev: {
                // SAFETY: `this` is a valid pointer to uninitialized memory.
                let inner = unsafe { &raw mut (*this.as_ptr()).inner };

                // SAFETY:
                // - `dev.as_raw()` is a pointer to a valid bound device.
                // - `inner` is guaranteed to be a valid for the duration of the lifetime of `Self`.
                // - `devm_add_action()` is guaranteed not to call `callback` until `this` has been
                //    properly initialized, because we require `dev` (i.e. the *bound* device) to
                //    live at least as long as the returned `impl PinInit<Self, Error>`.
                to_result(unsafe {
                    bindings::devm_add_action(dev.as_raw(), Some(callback), inner.cast())
                })?;

                dev.into()
            },
        })

I think you need to reorder the initialization of `inner` to be after
`dev` for this. And it should be fine, because the whole device is in
bound state while the PinInit being called, so `inner.cast()` being a
pointer to uninitialized memory should be fine (because the `callback`
won't be called).

Regards,
Boqun

> > Depending on how (1) is ensured, we might just need an unsafe function
> > that turns `Device<Normal>` into `Device<Bound>`.
> 
> `&Device<Normal>` in `&Device<Bound>`, yes. I have such a method locally
> already (but haven't sent it yet), because that's going to be a use-case for
> other abstractions as well. One specific example is the PWM Chip abstraction
> [1].
> 
> [1] https://lore.kernel.org/lkml/20250710-rust-next-pwm-working-fan-for-sending-v11-3-93824a16f9ec@samsung.com/