linux-kernel - Re: [PATCH] rust: irq: add support for request

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] [day] [month] [year] [list]

Message-ID: <CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com>
Date: Thu, 16 Jan 2025 09:29:44 +0100
From: Alice Ryhl <aliceryhl@...gle.com>
To: Daniel Almeida <daniel.almeida@...labora.com>
Cc: Gary Guo <gary@...yguo.net>, Miguel Ojeda <ojeda@...nel.org>, 
	Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...nel.org>, 
	Trevor Gross <tmgross@...ch.edu>, linux-kernel@...r.kernel.org, 
	rust-for-linux@...r.kernel.org
Subject: Re: [PATCH] rust: irq: add support for request_irq()

On Wed, Jan 15, 2025 at 1:31 PM Daniel Almeida
<daniel.almeida@...labora.com> wrote:
>
>
>
> > On 15 Jan 2025, at 08:38, Gary Guo <gary@...yguo.net> wrote:
> >
> > On Tue, 14 Jan 2025 15:57:57 -0300
> > Daniel Almeida <daniel.almeida@...labora.com> wrote:
> >
> >>>
> >>> It's not the pin_init! stuff, but the Opaque stuff. If it fails, then
> >>> it runs the destructor of Opaque<T>, which does *not* run the
> >>> destructor of T.
> >>>
> >>> Alice
> >>
> >> This is pretty unintuitive if you take into account trivial examples like
> >>
> >> ```
> >> struct Foo(T)
> >> ```
> >>
> >> Where dropping Foo drops T.
> >>
> >> Is there any reason why dropping Opaque<T> doesn’t behave similarly?
> >>
> >> — Daniel
> >
> > `Opaque` means that "this is a blob of bytes and don't touch it". It
> > can be uninitialized, so no meaningful action can be performed when
> > it's dropped.
> >
> > Best,
> > Gary
> >
>
> I really think that the documentation for this type has to be improved somehow.
>
> How is this a blob of bytes that can’t be touched, if it gives out a *mut T?
>
> I think I’ve consistently seen code that either accesses or mutates the inner T through
> the pointer.

It's not a problem to give out a *mut T. A Opaque is a blob of bytes
that *might* or *might not* contain a valid `T`. When you dereference
a raw pointer to the inner value, you are unsafely asserting that
*right now*, it does in fact contain a valid T.

The problem we're running into here is that Opaque is intended for
wrapping C types, but you're using it to wrap a Rust type instead. I
think we should have a new container type called `Aliased<T>` defined
like Opaque but without the MaybeUninit.

#[repr(transparent)]
pub struct Aliased<T> {
    value: UnsafeCell<T>,
    _pin: PhantomPinned,
}

This type can then serve the purpose of a Rust type that might be
accessed in weird ways, but *is* guaranteed to hold a valid Rust type.
It's destructor will run the destructor of T.

Alice