linux-kernel - Re: [PATCH v4 09/28] rust: alloc: implement kernel `Box`

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: <ZrNP9Lx-sTupwqG1@pollux>
Date: Wed, 7 Aug 2024 12:44:04 +0200
From: Danilo Krummrich <dakr@...nel.org>
To: Benno Lossin <benno.lossin@...ton.me>
Cc: Alice Ryhl <aliceryhl@...gle.com>, ojeda@...nel.org,
	alex.gaynor@...il.com, wedsonaf@...il.com, boqun.feng@...il.com,
	gary@...yguo.net, bjorn3_gh@...tonmail.com, a.hindborg@...sung.com,
	akpm@...ux-foundation.org, daniel.almeida@...labora.com,
	faith.ekstrand@...labora.com, boris.brezillon@...labora.com,
	lina@...hilina.net, mcanal@...lia.com, zhiw@...dia.com,
	acurrid@...dia.com, cjia@...dia.com, jhubbard@...dia.com,
	airlied@...hat.com, ajanulgu@...hat.com, lyude@...hat.com,
	linux-kernel@...r.kernel.org, rust-for-linux@...r.kernel.org,
	linux-mm@...ck.org
Subject: Re: [PATCH v4 09/28] rust: alloc: implement kernel `Box`

On Wed, Aug 07, 2024 at 08:01:00AM +0000, Benno Lossin wrote:
> On 07.08.24 09:51, Alice Ryhl wrote:
> > On Wed, Aug 7, 2024 at 9:49 AM Benno Lossin <benno.lossin@...ton.me> wrote:
> >> On 07.08.24 01:01, Danilo Krummrich wrote:
> >>> On Tue, Aug 06, 2024 at 07:47:17PM +0000, Benno Lossin wrote:
> >>>> On 05.08.24 17:19, Danilo Krummrich wrote:
> >>>>> +impl<T, A> Box<T, A>
> >>>>> +where
> >>>>> +    T: ?Sized,
> >>>>> +    A: Allocator,
> >>>>> +{
> >>>>> +    /// Constructs a `Box<T, A>` from a raw pointer.
> >>>>> +    ///
> >>>>> +    /// # Safety
> >>>>> +    ///
> >>>>> +    /// `raw` must point to valid memory, previously allocated with `A`, and at least the size of
> >>>>> +    /// type `T`.
> >>>>
> >>>> With this requirement and the invariant on `Box`, I am lead to believe
> >>>> that you can't use this for ZSTs, since they are not allocated with `A`.
> >>>> One solution would be to adjust this requirement. But I would rather use
> >>>> a different solution: we move the dangling pointer stuff into the
> >>>> allocator and also call it when `T` is a ZST (ie don't special case them
> >>>> in `Box` but in the impls of `Allocator`). That way this can stay as-is
> >>>> and the part about ZSTs in the invariant can be removed.
> >>>
> >>> Actually, we already got that. Every zero sized allocation will return a
> >>> dangling pointer. However, we can't call `Allocator::free` with (any) dangling
> >>> pointer though.
> >>
> >> The last part is rather problematic in my opinion, since the safety
> >> requirements of the functions in `Allocator` don't ensure that you're
> >> not allowed to do it. We should make it possible to free dangling
> >> pointers that were previously "allocated" by the allocator (ie returned
> >> by `realloc`).
> >> Maybe we do need an `old_layout` parameter for that (that way we can
> >> also `debug_assert_eq!(old_layout.align(), new_layout.align())`).
> > 
> > The std allocators generally prohibit zero sized allocations, so it
> > seems sensible for us to do the same?
> 
> I never understood why they do that, the stdlib `Allocator` trait has
> all the information it needs to detect zero-sized allocations, so it
> could just return dangling pointers. I don't see the point of
> duplicating the zero-sized logic in `Box` and `Vec`...

I think it's simpler and less error prone.

Besides that, I think the stdlib `Box` allows to call `from_raw` with any random
pointer for ZST, which the allocator API can't catch.

> 
> ---
> Cheers,
> Benno
>