linux-kernel - Re: [PATCH v6 09/26] 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: <b80cb238-5fcc-4bbb-8b03-42e173c28653@proton.me>
Date: Wed, 11 Sep 2024 13:26:32 +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 v6 09/26] rust: alloc: implement kernel `Box`

On 11.09.24 13:02, Danilo Krummrich wrote:
> On Wed, Sep 11, 2024 at 08:36:38AM +0000, Benno Lossin wrote:
>> On 11.09.24 01:25, Danilo Krummrich wrote:
>>> On Tue, Sep 10, 2024 at 07:49:42PM +0000, Benno Lossin wrote:
>>>> On 10.09.24 19:40, Danilo Krummrich wrote:
>>>>> On Sat, Aug 31, 2024 at 05:39:07AM +0000, Benno Lossin wrote:
>>>>>> On 16.08.24 02:10, Danilo Krummrich wrote:
>>>>>>> +///
>>>>>>> +/// ```
>>>>>>> +/// # use kernel::bindings;
>>>>>>> +/// const SIZE: usize = bindings::KMALLOC_MAX_SIZE as usize + 1;
>>>>>>> +/// struct Huge([u8; SIZE]);
>>>>>>> +///
>>>>>>> +/// assert!(KVBox::<Huge>::new_uninit(GFP_KERNEL).is_ok());
>>>>>>> +/// ```
>>>>>>
>>>>>> Similarly, you could then say above this one "Instead use either `VBox`
>>>>>> or `KVBox`:"
>>>>>>
>>>>>>> +///
>>>>>>> +/// # Invariants
>>>>>>> +///
>>>>>>> +/// The [`Box`]' pointer is always properly aligned and either points to memory allocated with `A`
>>>>>>
>>>>>> Please use `self.0` instead of "[`Box`]'".
>>>>>>
>>>>>>> +/// or, for zero-sized types, is a dangling pointer.
>>>>>>
>>>>>> Probably "dangling, well aligned pointer.".
>>>>>
>>>>> Does this add any value? For ZSTs everything is "well aligned", isn't it?
>>>>
>>>> ZSTs can have alignment and then unaligned pointers do exist for them
>>>> (and dereferencing them is UB!):
>>>
>>> Where is this documented? The documentation says:
>>>
>>> "For operations of size zero, *every* pointer is valid, including the null
>>> pointer. The following points are only concerned with non-zero-sized accesses."
>>> [1]
>>
>> That's a good point, the documentation looks a bit outdated. I found
>> this page in the nomicon: https://doc.rust-lang.org/nomicon/vec/vec-zsts.html
>> The first iterator implementation has an alignment issue. (Nevertheless,
>> that chapter of the nomicon is probably useful to you, since it goes
>> over implementing `Vec`, but maybe you already saw it)
>>
>>> [1] https://doc.rust-lang.org/std/ptr/index.html
>>
>> Might be a good idea to improve/complain about this at the rust project.
> 
> Well, my point is how do we know? There's no language specification and the
> documentation is (at least) ambiguous.

So I went through the unsafe-coding-guidelines issues list and only
found this one: https://github.com/rust-lang/unsafe-code-guidelines/issues/93
Maybe I missed something. You could also try to ask at the rust zulip in
the t-opsem channel for further clarification.

I think we should just be on the safe side and assume that ZSTs require
alignment. But if you get a convincing answer and if they say that they
will document it, then I don't mind removing the alignment requirement.

---
Cheers,
Benno