linux-kernel - Re: [PATCH v8 2/2] rust: add dma coherent allocator abstraction.

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: <588af6c5-02cf-437a-a909-c22744a6df1c@sedlak.dev>
Date: Wed, 8 Jan 2025 19:08:29 +0100
From: Daniel Sedlak <daniel@...lak.dev>
To: Abdiel Janulgue <abdiel.janulgue@...il.com>,
 daniel.almeida@...labora.com, aliceryhl@...gle.com, robin.murphy@....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>,
 Benno Lossin <benno.lossin@...ton.me>,
 Andreas Hindborg <a.hindborg@...nel.org>, Trevor Gross <tmgross@...ch.edu>,
 Danilo Krummrich <dakr@...nel.org>, Valentin Obst <kernel@...entinobst.de>,
 open list <linux-kernel@...r.kernel.org>, Christoph Hellwig <hch@....de>,
 Marek Szyprowski <m.szyprowski@...sung.com>, airlied@...hat.com,
 "open list:DMA MAPPING HELPERS" <iommu@...ts.linux.dev>
Subject: Re: [PATCH v8 2/2] rust: add dma coherent allocator abstraction.

On 1/8/25 1:27 PM, Abdiel Janulgue wrote:
> +/// Possible attributes associated with a DMA mapping.
> +///
> +/// They can be combined with the operators `|`, `&`, and `!`.
> +///
> +/// Values can be used from the [`attrs`] module.
> +#[derive(Clone, Copy, PartialEq)]
> +pub struct Attrs(pub u32);

Shouldn't this be non-pub? It seems unfortunate to leak C representation 
to the Rust world and allow its construction without going through 
constructor method. Also #[repr(transparent)] may be well suited? [1]

Link: https://doc.rust-lang.org/nomicon/other-reprs.html#reprtransparent 
[1]
> +
> +impl Attrs {
> +    /// Get the raw representation of this attribute.
> +    pub(crate) fn as_raw(self) -> usize {
> +        self.0.try_into().unwrap()
> +    }

Shouldn't be the return type u32 instead of usize? Or at least to my 
understanding the raw means C representation which should be the type 
that bindgen generates.
> +
> +    /// Check whether `flags` is contained in `self`.
> +    pub fn contains(self, flags: Attrs) -> bool {
> +        (self & flags) == flags
> +    }
> +}
Daniel