linux-kernel - Re: [PATCH v5 06/26] rust: alloc: implement `Vmalloc` allocator

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: <c693cfad-f3e4-48e8-820c-16bc3d9f46ab@proton.me>
Date: Thu, 15 Aug 2024 19:08:02 +0000
From: Benno Lossin <benno.lossin@...ton.me>
To: Danilo Krummrich <dakr@...nel.org>
Cc: 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, 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 v5 06/26] rust: alloc: implement `Vmalloc` allocator

On 15.08.24 16:23, Danilo Krummrich wrote:
> On Thu, Aug 15, 2024 at 01:44:27PM +0000, Benno Lossin wrote:
>> On 15.08.24 14:29, Danilo Krummrich wrote:
>>> On Thu, Aug 15, 2024 at 06:48:19AM +0000, Benno Lossin wrote:
>>>> On 15.08.24 01:20, Danilo Krummrich wrote:
>>>>> On Thu, Aug 15, 2024 at 12:13:06AM +0200, Danilo Krummrich wrote:
>>>>>>> How difficult will it be to support this? (it is a weird requirement,
>>>>>>> but I dislike just returning an error...)
>>>>>>
>>>>>> It's not difficult to support at all. But it requires a C API taking an
>>>>>> alignment argument (same for `KVmalloc`).
>>>>
>>>> I see, that's good to know.
>>>>
>>>>>> Coming up with a vrealloc_aligned() is rather trivial. kvrealloc_aligned() would
>>>>>> be a bit weird though, because the alignment argument could only be really
>>>>>> honored if we run into the vrealloc() case. For the krealloc() case it'd still
>>>>>> depend on the bucket size that is selected for the requested size.
>>>>
>>>> Yeah... Maybe some more logic on the Rust side can help with that.
>>>
>>> Only if we reimplement `KVmalloc` in Rust, However, there are quite some special
>>> cases in __kvmalloc_node_noprof(), i.e. fixup page flags, sanity check the size
>>> on kmalloc failure, fail on certain page flags, etc.
>>>
>>> I don't really want to duplicate this code, unless we absolutely have to.
>>
>> I am under the (probably wrong) impression that kvmalloc has some size
>> check and selects vmalloc or kmalloc depending on that.
> 
> Basically, yes. But as mentioned above, there are quite some corner cases [1].
> 
>> I think that we
>> could check the size and if it is going to allocate via kmalloc, then we
>> adjust the size for alignment as usual
> 
> We don't need this adjustment any longer, see commit ad59baa31695 ("slab, rust:
> extend kmalloc() alignment guarantees to remove Rust padding").
> 
>> and if it is going to select
>> vmalloc, then we can just pass the alignment (if the vmalloc alignment
>> patch is done first).
> 
> Yeah, but as mentioned, I'd prefer to do this in C, such that we don't need to
> open code everything the C code already does.
> 
> [1] https://elixir.bootlin.com/linux/v6.11-rc3/source/mm/util.c#L628

I see, then it's probably better to just add an align parameter variant
on the C side. Instead of rebuilding it in Rust.

---
Cheers,
Benno