linux-kernel - Re: [PATCH v19 1/3] rust: types: add `ForeignOwnable::PointedTo`

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAJ-ks9nrrKvbfjt-6RPk0G-qENukWDvw=6ePPxyBS-me-joTcw@mail.gmail.com>
Date: Wed, 30 Apr 2025 11:57:09 -0700
From: Tamir Duberstein <tamird@...il.com>
To: Gary Guo <gary@...yguo.net>
Cc: Danilo Krummrich <dakr@...nel.org>, 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>, 
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>, 
	Matthew Wilcox <willy@...radead.org>, Bjorn Helgaas <bhelgaas@...gle.com>, 
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	FUJITA Tomonori <fujita.tomonori@...il.com>, "Rob Herring (Arm)" <robh@...nel.org>, 
	Maíra Canal <mcanal@...lia.com>, 
	Asahi Lina <lina@...hilina.net>, rust-for-linux@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-pci@...r.kernel.org
Subject: Re: [PATCH v19 1/3] rust: types: add `ForeignOwnable::PointedTo`

On Wed, Apr 30, 2025 at 11:31 AM Gary Guo <gary@...yguo.net> wrote:
>
> On Wed, 23 Apr 2025 09:54:37 -0400
> Tamir Duberstein <tamird@...il.com> wrote:
>
> > Allow implementors to specify the foreign pointer type; this exposes
> > information about the pointed-to type such as its alignment.
> >
> > This requires the trait to be `unsafe` since it is now possible for
> > implementors to break soundness by returning a misaligned pointer.
> >
> > Encoding the pointer type in the trait (and avoiding pointer casts)
> > allows the compiler to check that implementors return the correct
> > pointer type. This is preferable to directly encoding the alignment in
> > the trait using a constant as the compiler would be unable to check it.
> >
> > Acked-by: Danilo Krummrich <dakr@...nel.org>
> > Signed-off-by: Tamir Duberstein <tamird@...il.com>
> > ---
> >  rust/kernel/alloc/kbox.rs | 38 ++++++++++++++++++++------------------
> >  rust/kernel/miscdevice.rs | 10 +++++-----
> >  rust/kernel/pci.rs        |  2 +-
> >  rust/kernel/platform.rs   |  2 +-
> >  rust/kernel/sync/arc.rs   | 21 ++++++++++++---------
> >  rust/kernel/types.rs      | 46 +++++++++++++++++++++++++++++++---------------
> >  6 files changed, 70 insertions(+), 49 deletions(-)
> >
> > diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
> > index b77d32f3a58b..6aa88b01e84d 100644
> > --- a/rust/kernel/alloc/kbox.rs
> > +++ b/rust/kernel/alloc/kbox.rs
> > @@ -360,68 +360,70 @@ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>
> >      }
> >  }
> >
> > -impl<T: 'static, A> ForeignOwnable for Box<T, A>
> > +// SAFETY: The `into_foreign` function returns a pointer that is well-aligned.
> > +unsafe impl<T: 'static, A> ForeignOwnable for Box<T, A>
> >  where
> >      A: Allocator,
> >  {
> > +    type PointedTo = T;
>
> I don't think this is the correct solution for this. The returned
> pointer is supposed to opaque, and exposing this type may encourage
> this is to be wrongly used.

Can you give an example?

> IIUC, the only reason for this to be exposed is for XArray to be able
> to check alignment. However `align_of::<PointedTo>()` is not the
> minimum guaranteed alignment.
>
> For example, if the type is allocated via kernel allocator then it can
> always guarantee to be at least usize-aligned. ZST can be arbitrarily
> aligned so ForeignOwnable` implementation could return a
> validly-aligned pointer for XArray. Actually, I think all current
> ForeignOwnable implementation can be modified to always give 4-byte
> aligned pointers.

Is your concern that this is *too restrictive*? It might be that the
minimum alignment is greater than `align_of::<PointedTo>()`, but not
less.

> Having a const associated item indicating the minimum guaranteed
> alignment for *that specific container* is the correct way IMO, not to
> reason about the pointee type!

How do you determine the value? You're at quite some distance from the
allocator implementation.