linux-kernel - Re: [PATCH v2 3/3] rust: kernel: add SgTable 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]

Date:   Fri, 2 Jun 2023 17:47:14 -0300
From:   Martin Rodriguez Reboredo <yakoyoku@...il.com>
To:     Qingsong Chen <changxian.cqs@...group.com>,
        linux-kernel@...r.kernel.org
Cc:     田洪亮 <tate.thl@...group.com>,
        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>,
        Benno Lossin <benno.lossin@...ton.me>,
        rust-for-linux@...r.kernel.org
Subject: Re: [PATCH v2 3/3] rust: kernel: add SgTable abstraction

On 6/2/23 07:18, Qingsong Chen wrote:
> SgTable is similar to `struct sg_table`, consisted of
> scatterlist entries, and could be chained with each other.
> 
> Signed-off-by: Qingsong Chen <changxian.cqs@...group.com>
> ---
> [...]
> +
> +impl<'a, const N: usize> SgTable<'a, N> {
> [...]
> +
> +    /// Chain two [`SgTable`] together.
> +    ///
> +    /// Transfer the last entry of this table as a chainable pointer to the
> +    /// first entry of `sgt` SgTable.
> +    pub fn chain_sgt<const M: usize>(&mut self, sgt: Pin<&mut SgTable<'_, M>>) -> Result {
> +        if self.count() != N - 1 {
> +            return Err(EINVAL);
> +        }
> +        self.entries[N - 2].unmark_end();
> +
> +        let next = ScatterList::as_mut(sgt.entries[0].opaque.get()).ok_or(EINVAL)?;
> +        self.entries[N - 1].chain_sgl(next);
> +        Ok(())
> +    }
> +
> +    /// Chain [`SgTable`] and [`ScatterList`] together.
> +    ///
> +    /// Transfer the last entry of this table as a chainable pointer to `sgl`
> +    /// scatterlist.
> +    pub fn chain_sgl(&mut self, sgl: Pin<&mut ScatterList<'_>>) -> Result {
> +        if self.count() != N - 1 {
> +            return Err(EINVAL);
> +        }
> +        self.entries[N - 2].unmark_end();
> +        self.entries[N - 1].chain_sgl(sgl);
> +        Ok(())
> +    }
> +
> +    /// Split the first table from chained scatterlist.
> +    pub fn split(&mut self) -> Result {
> +        if !self.entries[N - 1].is_chain() {
> +            return Err(EINVAL);
> +        }
> +        self.entries[N - 2].mark_end();
> +        Ok(())
> +    }

To these methods I'd suggest adding an `# Errors` section in the doc
comment to explain why would you get an `EINVAL`.

> +
> [...]
> +
> +    /// Get an iterator for immutable entries.
> +    pub fn iter(&self) -> Iter<'_> {
> +        Iter(ScatterList::as_ref(self.entries[0].opaque.get()))
> +    }
> +
> +    /// Get an iterator for mutable entries.
> +    pub fn iter_mut(&mut self) -> IterMut<'_> {
> +        IterMut(ScatterList::as_mut(self.entries[0].opaque.get()))
> +    }

Same as the previous commit, the `SAFETY` comment.

> +}