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: <Z407egxOy7oNLpq8@boqun-archlinux>
Date: Sun, 19 Jan 2025 09:50:50 -0800
From: Boqun Feng <boqun.feng@...il.com>
To: Alice Ryhl <aliceryhl@...gle.com>
Cc: Daniel Almeida <daniel.almeida@...labora.com>,
	Gary Guo <gary@...yguo.net>, Miguel Ojeda <ojeda@...nel.org>,
	Alex Gaynor <alex.gaynor@...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 Thu, Jan 16, 2025 at 09:29:44AM +0100, Alice Ryhl wrote:
> 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> {

Hmm.. I feel that `Aliased` is a name that reflects too much more
compiler/backend details other than programmers' need. That said, the
best I can come up now is `Shared<T>`, which indicates the field is
always shared therefore even a &mut could not assume noalias.

Regards,
Boqun

>     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