linux-kernel - Re: [PATCH v7 01/26] rust: alloc: add `Allocator` trait

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: <ZudTon205iOCdQbq@pollux>
Date: Sun, 15 Sep 2024 23:37:38 +0200
From: Danilo Krummrich <dakr@...nel.org>
To: Gary Guo <gary@...yguo.net>
Cc: ojeda@...nel.org, alex.gaynor@...il.com, wedsonaf@...il.com,
	boqun.feng@...il.com, bjorn3_gh@...tonmail.com,
	benno.lossin@...ton.me, a.hindborg@...sung.com,
	aliceryhl@...gle.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, 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 v7 01/26] rust: alloc: add `Allocator` trait

On Sun, Sep 15, 2024 at 08:22:42PM +0100, Gary Guo wrote:
> On Sun, 15 Sep 2024 19:02:40 +0200
> Danilo Krummrich <dakr@...nel.org> wrote:
> > There is also 4.
> > 
> > Let `alloc` and `realloc` return a properly aligned dangling pointer for
> > `size == 0` and don't accept dangling pointers in `realloc` and `free`.
> 
> I'll consider the API design to be bad if I can't pass allocated pointer to
> free. If caller needs to handle ZST specially then we might as well
> just ban it completely.

Fine for me -- I don't see a need to support ZSTs with `Allocator`.

I think its main purpose is to give us an interface to actually allocate memory.
We probably don't need it to be a "dangling pointer generator".

> 
> > And 5.
> > 
> > Reject the combination of `None` and `size == 0` entirely, as earlier proposed
> > by Benno.
> > 
> > I'm fine with both, 4. and 5. with a slight preference for 4.
> > 
> > I'd also go along with 1., as a mix of 4. and 5.
> > 
> > I really don't like making `alloc` unsafe, and I really don't want to have
> > `old_layout` in `free`. Please let's not discuss this again. :-)
> 
> I don't buy it.
> 
> Your argument for having `old_layout` is so that the caller doesn't
> need to care about the size. But as demonstrated the caller *does* need
> to care about whether the size is zero.
> 
> Our previous discussion doesn't cover the particular case of ZST and
> you said that it reason arise that we need this extra parameter, then
> it could be added. It feels to me that sane behaviour when it comes
> to ZST allocation is a very good reason.

I don't see why we should "optimize" the API for creating dangling pointers and
be able to pass them to `free` (which does not serve any practical purpose).

I don't want to add arguments that are meaningless for the actual backing
allocators (such as Kmalloc, Vmalloc, etc.), only to be able to generate and
"free" pointers for ZSTs with arbitrary alignment.

Do we even have use cases for ZSTs with other alignments?

> 
> > 
> > > 
> > > Best,
> > > Gary
> > >   
> > > > 
> > > > Reviewed-by: Alice Ryhl <aliceryhl@...gle.com>
> > > > Signed-off-by: Danilo Krummrich <dakr@...nel.org>
> > > > ---
> > > >  rust/kernel/alloc.rs | 112 +++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 112 insertions(+)
>