linux-kernel - Re: [PATCH v3 07/10] rust: alloc: update `VecExt` to take allocation flags

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: <59c5bda5-6116-42f1-bf8e-47eba02e2ecb@proton.me>
Date: Sat, 30 Mar 2024 13:30:17 +0000
From: Benno Lossin <benno.lossin@...ton.me>
To: Wedson Almeida Filho <wedsonaf@...il.com>, rust-for-linux@...r.kernel.org
Cc: Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, Björn Roy Baron <bjorn3_gh@...tonmail.com>, Andreas Hindborg <a.hindborg@...sung.com>, Alice Ryhl <aliceryhl@...gle.com>, linux-kernel@...r.kernel.org, Wedson Almeida Filho <walmeida@...rosoft.com>
Subject: Re: [PATCH v3 07/10] rust: alloc: update `VecExt` to take allocation flags

On 28.03.24 02:36, Wedson Almeida Filho wrote:
> From: Wedson Almeida Filho <walmeida@...rosoft.com>
> 
> We also rename the methods by removing the `try_` prefix since the names
> are available due to our usage of the `no_global_oom_handling` config
> when building the `alloc` crate.
> 
> Reviewed-by: Boqun Feng <boqun.feng@...il.com>
> Signed-off-by: Wedson Almeida Filho <walmeida@...rosoft.com>
> ---
>   rust/kernel/alloc/vec_ext.rs | 158 +++++++++++++++++++++++++++++++----
>   rust/kernel/error.rs         |  11 +--
>   rust/kernel/lib.rs           |   1 -
>   rust/kernel/str.rs           |   6 +-
>   rust/kernel/types.rs         |   4 +-
>   samples/rust/rust_minimal.rs |   6 +-
>   6 files changed, 152 insertions(+), 34 deletions(-)

Reviewed-by: Benno Lossin <benno.lossin@...ton.me>

[...]

>   impl<T> VecExt<T> for Vec<T> {
> -    fn try_with_capacity(capacity: usize) -> Result<Self, TryReserveError> {
> +    fn with_capacity(capacity: usize, flags: Flags) -> Result<Self, AllocError> {
>           let mut v = Vec::new();
> -        v.try_reserve(capacity)?;
> +        <Self as VecExt<_>>::reserve(&mut v, capacity, flags)?;
>           Ok(v)
>       }
> 
> -    fn try_push(&mut self, v: T) -> Result<(), TryReserveError> {
> -        if let Err(retry) = self.push_within_capacity(v) {
> -            self.try_reserve(1)?;
> -            let _ = self.push_within_capacity(retry);
> -        }
> +    fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError> {
> +        <Self as VecExt<_>>::reserve(self, 1, flags)?;
> +        let s = self.spare_capacity_mut();
> +        s[0].write(v);

I haven't checked the codegen, but this could result in an additional
bound check. We don't need to care about this now, but at some point we
should do a performance test. There are probably other things that I
have missed.

-- 
Cheers,
Benno

> +
> +        // SAFETY: We just initialised the first spare entry, so it is safe to increase the length
> +        // by 1. We also know that the new length is <= capacity because of the previous call to
> +        // `reserve` above.
> +        unsafe { self.set_len(self.len() + 1) };
>           Ok(())
>       }

[...]