linux-kernel - Re: [PATCH v3 04/13] rust: add pin-init API core

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: <ce17f68e-521f-f55e-8ae2-35bcd6ebd709@ryhl.io>
Date:   Thu, 30 Mar 2023 15:33:44 +0200
From:   Alice Ryhl <alice@...l.io>
To:     y86-dev@...tonmail.com
Cc:     rust-for-linux@...r.kernel.org, linux-kernel@...r.kernel.org,
        patches@...ts.linux.dev, Miguel Ojeda <ojeda@...nel.org>,
        Alex Gaynor <alex.gaynor@...il.com>,
        Wedson Almeida Filho <wedsonaf@...il.com>,
        Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>,
        Björn Roy Baron <bjorn3_gh@...tonmail.com>
Subject: Re: [PATCH v3 04/13] rust: add pin-init API core

On 3/30/23 00:33, y86-dev@...tonmail.com wrote:
> From: Benno Lossin <y86-dev@...tonmail.com>
> 
> This API is used to facilitate safe pinned initialization of structs. It
> replaces cumbersome `unsafe` manual initialization with elegant safe macro
> invocations.
> 
> Due to the size of this change it has been split into six commits:
> 1. This commit introducing the basic public interface: traits and
>     functions to represent and create initializers.
> 2. Adds the `#[pin_data]`, `pin_init!`, `try_pin_init!`, `init!` and
>     `try_init!` macros along with their internal types.
> 3. Adds the `InPlaceInit` trait that allows using an initializer to create
>     an object inside of a `Box<T>` and other smart pointers.
> 4. Adds the `PinnedDrop` trait and adds macro support for it in
>     the `#[pin_data]` macro.
> 5. Adds the `stack_pin_init!` macro allowing to pin-initialize a struct on
>     the stack.
> 6. Adds the `Zeroable` trait and `init::zeroed` function to initialize
>     types that have `0x00` in all bytes as a valid bit pattern.
> 
> Co-developed-by: Gary Guo <gary@...yguo.net>
> Signed-off-by: Gary Guo <gary@...yguo.net>
> Signed-off-by: Benno Lossin <y86-dev@...tonmail.com>

Reviewed-by: Alice Ryhl <aliceryhl@...gle.com>

> +//! Aside from pinned initialization, this API also supports in-place construction without pinning,
> +//! the marcos/types/functions are generally named like the pinned variants without the `pin`
> +//! prefix.

Typo: Should be "macros".

> +type Invariant<T> = PhantomData<fn(*mut T) -> *mut T>;

I think it would make sense to include a link to the nomicon on the 
documentation for the Invariant type.

E.g. this link: https://doc.rust-lang.org/nomicon/subtyping.html

> +// This is the module-internal type implementing `PinInit` and `Init`. It is unsafe to create this
> +// type, since the closure needs to fulfill the same safety requirement as the
> +// `__pinned_init`/`__init` functions.
> +struct InitClosure<F, T: ?Sized, E>(F, Invariant<(E, T)>);

Documentation for a type should use /// rather than //.

I think it would help to call out explicitly in the documentation on 
this type that it is an implementation detail of the 
pin_init_from_closure and init_from_closure methods.