[RFC PATCH v2 0/2] Rust abstractions for Crypto API

[RFC PATCH v2 0/2] Rust abstractions for Crypto API

[FUJITA Tomonori]

Hi,

Before sending v2 of my crypto patch [1] to linux-crypto ml and
checking the chance of Rust bindings for crypto being accepted, I'd
like to iron out Rust issues. I'd appreciate any feedback.

Once reviewing on rust-for-linux ml is done, I'll send only crypto
support to linux-crypto (drops the socket support patch to netdev).

Changes since v1:
- addressed Eric's comments on hash support [2].
- add rng support
- split files

[1] https://lore.kernel.org/netdev/010101881db036fb-2fb6981d-e0ef-4ad1-83c3-54d64b6d93b3-000000@us-west-2.amazonses.com/T/
[2] https://www.spinics.net/lists/netdev/msg904380.html

FUJITA Tomonori (2):
  rust: add synchronous message digest support
  rust: add Random Number Generator algorithms support

 rust/bindings/bindings_helper.h |   2 +
 rust/helpers.c                  |  38 ++++++++++
 rust/kernel/crypto.rs           |   6 ++
 rust/kernel/crypto/hash.rs      | 118 ++++++++++++++++++++++++++++++++
 rust/kernel/crypto/rng.rs       |  72 +++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 6 files changed, 238 insertions(+)
 create mode 100644 rust/kernel/crypto.rs
 create mode 100644 rust/kernel/crypto/hash.rs
 create mode 100644 rust/kernel/crypto/rng.rs


base-commit: d2e3115d717197cb2bc020dd1f06b06538474ac3
-- 
2.34.1

[RFC PATCH v2 1/2] rust: add synchronous message digest support

[FUJITA Tomonori]

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers.c                  |  26 +++++++
 rust/kernel/crypto.rs           |   5 ++
 rust/kernel/crypto/hash.rs      | 118 ++++++++++++++++++++++++++++++++
 rust/kernel/lib.rs              |   2 +
 5 files changed, 152 insertions(+)
 create mode 100644 rust/kernel/crypto.rs
 create mode 100644 rust/kernel/crypto/hash.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 3e601ce2548d..2f198c6d5de5 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -6,6 +6,7 @@
  * Sorted alphabetically.
  */
 
+#include <crypto/hash.h>
 #include <linux/errname.h>
 #include <linux/slab.h>
 #include <linux/refcount.h>
diff --git a/rust/helpers.c b/rust/helpers.c
index bb594da56137..7966902ed8eb 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -18,6 +18,7 @@
  * accidentally exposed.
  */
 
+#include <crypto/hash.h>
 #include <linux/bug.h>
 #include <linux/build_bug.h>
 #include <linux/err.h>
@@ -28,6 +29,31 @@
 #include <linux/sched/signal.h>
 #include <linux/wait.h>
 
+#ifdef CONFIG_CRYPTO
+void rust_helper_crypto_free_shash(struct crypto_shash *tfm)
+{
+	crypto_free_shash(tfm);
+}
+EXPORT_SYMBOL_GPL(rust_helper_crypto_free_shash);
+
+unsigned int rust_helper_crypto_shash_digestsize(struct crypto_shash *tfm)
+{
+    return crypto_shash_digestsize(tfm);
+}
+EXPORT_SYMBOL_GPL(rust_helper_crypto_shash_digestsize);
+
+unsigned int rust_helper_crypto_shash_descsize(struct crypto_shash *tfm)
+{
+    return crypto_shash_descsize(tfm);
+}
+EXPORT_SYMBOL_GPL(rust_helper_crypto_shash_descsize);
+
+int rust_helper_crypto_shash_init(struct shash_desc *desc) {
+	return crypto_shash_init(desc);
+}
+EXPORT_SYMBOL_GPL(rust_helper_crypto_shash_init);
+#endif
+
 __noreturn void rust_helper_BUG(void)
 {
 	BUG();
diff --git a/rust/kernel/crypto.rs b/rust/kernel/crypto.rs
new file mode 100644
index 000000000000..f80dd7bd3381
--- /dev/null
+++ b/rust/kernel/crypto.rs
@@ -0,0 +1,5 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Cryptography.
+
+pub mod hash;
diff --git a/rust/kernel/crypto/hash.rs b/rust/kernel/crypto/hash.rs
new file mode 100644
index 000000000000..53f4a311b3b2
--- /dev/null
+++ b/rust/kernel/crypto/hash.rs
@@ -0,0 +1,118 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Cryptographic Hash operations.
+//!
+//! C headers: [`include/crypto/hash.h`](../../../../include/crypto/hash.h)
+
+use crate::{
+    error::{code::ENOMEM, from_err_ptr, to_result, Result},
+    str::CStr,
+};
+use alloc::alloc::{alloc, dealloc};
+use core::alloc::Layout;
+
+/// Corresponds to the kernel's `struct crypto_shash`.
+///
+/// # Invariants
+///
+/// The pointer is valid.
+pub struct Shash(*mut bindings::crypto_shash);
+
+impl Drop for Shash {
+    fn drop(&mut self) {
+        // SAFETY: The type invariant guarantees that the pointer is valid.
+        unsafe { bindings::crypto_free_shash(self.0) }
+    }
+}
+
+impl Shash {
+    /// Creates a [`Shash`] object for a message digest handle.
+    pub fn new(name: &CStr, t: u32, mask: u32) -> Result<Shash> {
+        // SAFETY: FFI call.
+        let ptr =
+            unsafe { from_err_ptr(bindings::crypto_alloc_shash(name.as_char_ptr(), t, mask)) }?;
+        // INVARIANT: `ptr` is valid and non-null since `crypto_alloc_shash`
+        // returned a valid pointer which was null-checked.
+        Ok(Self(ptr))
+    }
+
+    /// Sets optional key used by the hashing algorithm.
+    pub fn setkey(&mut self, data: &[u8]) -> Result {
+        // SAFETY: The type invariant guarantees that the pointer is valid.
+        to_result(unsafe {
+            bindings::crypto_shash_setkey(self.0, data.as_ptr(), data.len() as u32)
+        })
+    }
+
+    /// Returns the size of the result of the transformation.
+    pub fn digestsize(&self) -> u32 {
+        // SAFETY: The type invariant guarantees that the pointer is valid.
+        unsafe { bindings::crypto_shash_digestsize(self.0) }
+    }
+}
+
+/// Corresponds to the kernel's `struct shash_desc`.
+///
+/// # Invariants
+///
+/// The field `ptr` is valid.
+pub struct ShashDesc<'a> {
+    ptr: *mut bindings::shash_desc,
+    tfm: &'a Shash,
+    size: usize,
+}
+
+impl Drop for ShashDesc<'_> {
+    fn drop(&mut self) {
+        // SAFETY: The type invariant guarantees that the pointer is valid.
+        unsafe {
+            dealloc(
+                self.ptr.cast(),
+                Layout::from_size_align(self.size, 2).unwrap(),
+            );
+        }
+    }
+}
+
+impl<'a> ShashDesc<'a> {
+    /// Creates a [`ShashDesc`] object for a request data structure for message digest.
+    pub fn new(tfm: &'a Shash) -> Result<Self> {
+        // SAFETY: The type invariant guarantees that `tfm.0` pointer is valid.
+        let size = core::mem::size_of::<bindings::shash_desc>()
+            + unsafe { bindings::crypto_shash_descsize(tfm.0) } as usize;
+        let layout = Layout::from_size_align(size, 2)?;
+        let ptr = unsafe { alloc(layout) } as *mut bindings::shash_desc;
+        if ptr.is_null() {
+            return Err(ENOMEM);
+        }
+        // INVARIANT: `ptr` is valid and non-null since `alloc`
+        // returned a valid pointer which was null-checked.
+        let mut desc = ShashDesc { ptr, tfm, size };
+        // SAFETY: `desc.ptr` is valid and non-null since `alloc`
+        // returned a valid pointer which was null-checked.
+        // Additionally, The type invariant guarantees that `tfm.0` is valid.
+        unsafe { (*desc.ptr).tfm = desc.tfm.0 };
+        desc.reset()?;
+        Ok(desc)
+    }
+
+    /// Re-initializes message digest.
+    pub fn reset(&mut self) -> Result {
+        // SAFETY: The type invariant guarantees that the pointer is valid.
+        to_result(unsafe { bindings::crypto_shash_init(self.ptr) })
+    }
+
+    /// Adds data to message digest for processing.
+    pub fn update(&mut self, data: &[u8]) -> Result {
+        // SAFETY: The type invariant guarantees that the pointer is valid.
+        to_result(unsafe {
+            bindings::crypto_shash_update(self.ptr, data.as_ptr(), data.len() as u32)
+        })
+    }
+
+    /// Calculates message digest.
+    pub fn finalize(&mut self, output: &mut [u8]) -> Result {
+        // SAFETY: The type invariant guarantees that the pointer is valid.
+        to_result(unsafe { bindings::crypto_shash_final(self.ptr, output.as_mut_ptr()) })
+    }
+}
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 85b261209977..3cb8bd8a17d9 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -31,6 +31,8 @@
 #[cfg(not(testlib))]
 mod allocator;
 mod build_assert;
+#[cfg(CONFIG_CRYPTO)]
+pub mod crypto;
 pub mod error;
 pub mod init;
 pub mod ioctl;
-- 
2.34.1

Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support

[Greg KH]

On Thu, Jun 15, 2023 at 11:23:10PM +0900, FUJITA Tomonori wrote:
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>

For obvious reasons, we can't take commits with no changelog text at
all, nor would you want us to :)

thanks,

greg k-h

Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support

[FUJITA Tomonori]

Hi,

On Thu, 15 Jun 2023 17:01:06 +0200
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Thu, Jun 15, 2023 at 11:23:10PM +0900, FUJITA Tomonori wrote:
>> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> 
> For obvious reasons, we can't take commits with no changelog text at
> all, nor would you want us to :)

Oops, definitely I'll write nice changelog in the next version.

thanks,

Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support

[Alex Gaynor]

One note inline

On Thu, Jun 15, 2023 at 10:50 AM FUJITA Tomonori
<fujita.tomonori@gmail.com> wrote:
>
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/bindings/bindings_helper.h |   1 +
>  rust/helpers.c                  |  26 +++++++
>  rust/kernel/crypto.rs           |   5 ++
>  rust/kernel/crypto/hash.rs      | 118 ++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs              |   2 +
>  5 files changed, 152 insertions(+)
>  create mode 100644 rust/kernel/crypto.rs
>  create mode 100644 rust/kernel/crypto/hash.rs
>
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 3e601ce2548d..2f198c6d5de5 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -6,6 +6,7 @@
>   * Sorted alphabetically.
>   */
>
> +#include <crypto/hash.h>
>  #include <linux/errname.h>
>  #include <linux/slab.h>
>  #include <linux/refcount.h>
> diff --git a/rust/helpers.c b/rust/helpers.c
> index bb594da56137..7966902ed8eb 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -18,6 +18,7 @@
>   * accidentally exposed.
>   */
>
> +#include <crypto/hash.h>
>  #include <linux/bug.h>
>  #include <linux/build_bug.h>
>  #include <linux/err.h>
> @@ -28,6 +29,31 @@
>  #include <linux/sched/signal.h>
>  #include <linux/wait.h>
>
> +#ifdef CONFIG_CRYPTO
> +void rust_helper_crypto_free_shash(struct crypto_shash *tfm)
> +{
> +       crypto_free_shash(tfm);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_crypto_free_shash);
> +
> +unsigned int rust_helper_crypto_shash_digestsize(struct crypto_shash *tfm)
> +{
> +    return crypto_shash_digestsize(tfm);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_crypto_shash_digestsize);
> +
> +unsigned int rust_helper_crypto_shash_descsize(struct crypto_shash *tfm)
> +{
> +    return crypto_shash_descsize(tfm);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_crypto_shash_descsize);
> +
> +int rust_helper_crypto_shash_init(struct shash_desc *desc) {
> +       return crypto_shash_init(desc);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_crypto_shash_init);
> +#endif
> +
>  __noreturn void rust_helper_BUG(void)
>  {
>         BUG();
> diff --git a/rust/kernel/crypto.rs b/rust/kernel/crypto.rs
> new file mode 100644
> index 000000000000..f80dd7bd3381
> --- /dev/null
> +++ b/rust/kernel/crypto.rs
> @@ -0,0 +1,5 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Cryptography.
> +
> +pub mod hash;
> diff --git a/rust/kernel/crypto/hash.rs b/rust/kernel/crypto/hash.rs
> new file mode 100644
> index 000000000000..53f4a311b3b2
> --- /dev/null
> +++ b/rust/kernel/crypto/hash.rs
> @@ -0,0 +1,118 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Cryptographic Hash operations.
> +//!
> +//! C headers: [`include/crypto/hash.h`](../../../../include/crypto/hash.h)
> +
> +use crate::{
> +    error::{code::ENOMEM, from_err_ptr, to_result, Result},
> +    str::CStr,
> +};
> +use alloc::alloc::{alloc, dealloc};
> +use core::alloc::Layout;
> +
> +/// Corresponds to the kernel's `struct crypto_shash`.
> +///
> +/// # Invariants
> +///
> +/// The pointer is valid.
> +pub struct Shash(*mut bindings::crypto_shash);
> +
> +impl Drop for Shash {
> +    fn drop(&mut self) {
> +        // SAFETY: The type invariant guarantees that the pointer is valid.
> +        unsafe { bindings::crypto_free_shash(self.0) }
> +    }
> +}
> +
> +impl Shash {
> +    /// Creates a [`Shash`] object for a message digest handle.
> +    pub fn new(name: &CStr, t: u32, mask: u32) -> Result<Shash> {
> +        // SAFETY: FFI call.
> +        let ptr =
> +            unsafe { from_err_ptr(bindings::crypto_alloc_shash(name.as_char_ptr(), t, mask)) }?;
> +        // INVARIANT: `ptr` is valid and non-null since `crypto_alloc_shash`
> +        // returned a valid pointer which was null-checked.
> +        Ok(Self(ptr))
> +    }
> +
> +    /// Sets optional key used by the hashing algorithm.
> +    pub fn setkey(&mut self, data: &[u8]) -> Result {
> +        // SAFETY: The type invariant guarantees that the pointer is valid.
> +        to_result(unsafe {
> +            bindings::crypto_shash_setkey(self.0, data.as_ptr(), data.len() as u32)
> +        })
> +    }
> +
> +    /// Returns the size of the result of the transformation.
> +    pub fn digestsize(&self) -> u32 {
> +        // SAFETY: The type invariant guarantees that the pointer is valid.
> +        unsafe { bindings::crypto_shash_digestsize(self.0) }
> +    }
> +}
> +
> +/// Corresponds to the kernel's `struct shash_desc`.
> +///
> +/// # Invariants
> +///
> +/// The field `ptr` is valid.
> +pub struct ShashDesc<'a> {
> +    ptr: *mut bindings::shash_desc,
> +    tfm: &'a Shash,
> +    size: usize,
> +}
> +
> +impl Drop for ShashDesc<'_> {
> +    fn drop(&mut self) {
> +        // SAFETY: The type invariant guarantees that the pointer is valid.
> +        unsafe {
> +            dealloc(
> +                self.ptr.cast(),
> +                Layout::from_size_align(self.size, 2).unwrap(),
> +            );
> +        }
> +    }
> +}
> +
> +impl<'a> ShashDesc<'a> {
> +    /// Creates a [`ShashDesc`] object for a request data structure for message digest.
> +    pub fn new(tfm: &'a Shash) -> Result<Self> {
> +        // SAFETY: The type invariant guarantees that `tfm.0` pointer is valid.
> +        let size = core::mem::size_of::<bindings::shash_desc>()
> +            + unsafe { bindings::crypto_shash_descsize(tfm.0) } as usize;
> +        let layout = Layout::from_size_align(size, 2)?;
> +        let ptr = unsafe { alloc(layout) } as *mut bindings::shash_desc;
> +        if ptr.is_null() {
> +            return Err(ENOMEM);
> +        }
> +        // INVARIANT: `ptr` is valid and non-null since `alloc`
> +        // returned a valid pointer which was null-checked.
> +        let mut desc = ShashDesc { ptr, tfm, size };
> +        // SAFETY: `desc.ptr` is valid and non-null since `alloc`
> +        // returned a valid pointer which was null-checked.
> +        // Additionally, The type invariant guarantees that `tfm.0` is valid.
> +        unsafe { (*desc.ptr).tfm = desc.tfm.0 };
> +        desc.reset()?;
> +        Ok(desc)
> +    }
> +
> +    /// Re-initializes message digest.
> +    pub fn reset(&mut self) -> Result {
> +        // SAFETY: The type invariant guarantees that the pointer is valid.
> +        to_result(unsafe { bindings::crypto_shash_init(self.ptr) })
> +    }
> +
> +    /// Adds data to message digest for processing.
> +    pub fn update(&mut self, data: &[u8]) -> Result {
> +        // SAFETY: The type invariant guarantees that the pointer is valid.
> +        to_result(unsafe {
> +            bindings::crypto_shash_update(self.ptr, data.as_ptr(), data.len() as u32)
> +        })
> +    }
> +
> +    /// Calculates message digest.
> +    pub fn finalize(&mut self, output: &mut [u8]) -> Result {
> +        // SAFETY: The type invariant guarantees that the pointer is valid.
> +        to_result(unsafe { bindings::crypto_shash_final(self.ptr, output.as_mut_ptr()) })

Is this sound? What happens if output.len() < the digest size?

> +    }
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 85b261209977..3cb8bd8a17d9 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -31,6 +31,8 @@
>  #[cfg(not(testlib))]
>  mod allocator;
>  mod build_assert;
> +#[cfg(CONFIG_CRYPTO)]
> +pub mod crypto;
>  pub mod error;
>  pub mod init;
>  pub mod ioctl;
> --
> 2.34.1
>


-- 
All that is necessary for evil to succeed is for good people to do nothing.

Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support

[FUJITA Tomonori]

Hi,

On Thu, 15 Jun 2023 11:02:31 -0400
Alex Gaynor <alex.gaynor@gmail.com> wrote:

>> +    /// Calculates message digest.
>> +    pub fn finalize(&mut self, output: &mut [u8]) -> Result {
>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>> +        to_result(unsafe { bindings::crypto_shash_final(self.ptr, output.as_mut_ptr()) })
> 
> Is this sound? What happens if output.len() < the digest size?

Good point, surely we need that checking before calling crypto_shash_final().

It will be fixed in the next version.

Thanks a lot!

Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support

[Benno Lossin]

> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/bindings/bindings_helper.h |   1 +
>  rust/helpers.c                  |  26 +++++++
>  rust/kernel/crypto.rs           |   5 ++
>  rust/kernel/crypto/hash.rs      | 118 ++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs              |   2 +
>  5 files changed, 152 insertions(+)
>  create mode 100644 rust/kernel/crypto.rs
>  create mode 100644 rust/kernel/crypto/hash.rs
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 3e601ce2548d..2f198c6d5de5 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -6,6 +6,7 @@
>   * Sorted alphabetically.
>   */
> 
> +#include <crypto/hash.h>
>  #include <linux/errname.h>
>  #include <linux/slab.h>
>  #include <linux/refcount.h>
> diff --git a/rust/helpers.c b/rust/helpers.c
> index bb594da56137..7966902ed8eb 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -18,6 +18,7 @@
>   * accidentally exposed.
>   */
> 
> +#include <crypto/hash.h>
>  #include <linux/bug.h>
>  #include <linux/build_bug.h>
>  #include <linux/err.h>
> @@ -28,6 +29,31 @@
>  #include <linux/sched/signal.h>
>  #include <linux/wait.h>
> 
> +#ifdef CONFIG_CRYPTO
> +void rust_helper_crypto_free_shash(struct crypto_shash *tfm)
> +{
> +	crypto_free_shash(tfm);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_crypto_free_shash);
> +
> +unsigned int rust_helper_crypto_shash_digestsize(struct crypto_shash *tfm)
> +{
> +    return crypto_shash_digestsize(tfm);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_crypto_shash_digestsize);
> +
> +unsigned int rust_helper_crypto_shash_descsize(struct crypto_shash *tfm)
> +{
> +    return crypto_shash_descsize(tfm);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_crypto_shash_descsize);
> +
> +int rust_helper_crypto_shash_init(struct shash_desc *desc) {
> +	return crypto_shash_init(desc);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_crypto_shash_init);
> +#endif
> +
>  __noreturn void rust_helper_BUG(void)
>  {
>  	BUG();
> diff --git a/rust/kernel/crypto.rs b/rust/kernel/crypto.rs
> new file mode 100644
> index 000000000000..f80dd7bd3381
> --- /dev/null
> +++ b/rust/kernel/crypto.rs
> @@ -0,0 +1,5 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Cryptography.
> +
> +pub mod hash;
> diff --git a/rust/kernel/crypto/hash.rs b/rust/kernel/crypto/hash.rs
> new file mode 100644
> index 000000000000..53f4a311b3b2
> --- /dev/null
> +++ b/rust/kernel/crypto/hash.rs
> @@ -0,0 +1,118 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Cryptographic Hash operations.
> +//!
> +//! C headers: [`include/crypto/hash.h`](../../../../include/crypto/hash.h)
> +
> +use crate::{
> +    error::{code::ENOMEM, from_err_ptr, to_result, Result},
> +    str::CStr,
> +};
> +use alloc::alloc::{alloc, dealloc};
> +use core::alloc::Layout;
> +
> +/// Corresponds to the kernel's `struct crypto_shash`.
> +///
> +/// # Invariants
> +///
> +/// The pointer is valid.
> +pub struct Shash(*mut bindings::crypto_shash);
> +
> +impl Drop for Shash {
> +    fn drop(&mut self) {
> +        // SAFETY: The type invariant guarantees that the pointer is valid.
> +        unsafe { bindings::crypto_free_shash(self.0) }
> +    }
> +}
> +
> +impl Shash {
> +    /// Creates a [`Shash`] object for a message digest handle.
> +    pub fn new(name: &CStr, t: u32, mask: u32) -> Result<Shash> {
> +        // SAFETY: FFI call.

If there are no requirements then say so.

> +        let ptr =
> +            unsafe { from_err_ptr(bindings::crypto_alloc_shash(name.as_char_ptr(), t, mask)) }?;
> +        // INVARIANT: `ptr` is valid and non-null since `crypto_alloc_shash`
> +        // returned a valid pointer which was null-checked.
> +        Ok(Self(ptr))
> +    }
> +
> +    /// Sets optional key used by the hashing algorithm.
> +    pub fn setkey(&mut self, data: &[u8]) -> Result {

This should be called `set_key`.

> +        // SAFETY: The type invariant guarantees that the pointer is valid.
> +        to_result(unsafe {
> +            bindings::crypto_shash_setkey(self.0, data.as_ptr(), data.len() as u32)
> +        })
> +    }
> +
> +    /// Returns the size of the result of the transformation.
> +    pub fn digestsize(&self) -> u32 {

This should be called `digest_size`.

> +        // SAFETY: The type invariant guarantees that the pointer is valid.
> +        unsafe { bindings::crypto_shash_digestsize(self.0) }
> +    }
> +}
> +
> +/// Corresponds to the kernel's `struct shash_desc`.
> +///
> +/// # Invariants
> +///
> +/// The field `ptr` is valid.
> +pub struct ShashDesc<'a> {
> +    ptr: *mut bindings::shash_desc,
> +    tfm: &'a Shash,
> +    size: usize,
> +}
> +
> +impl Drop for ShashDesc<'_> {
> +    fn drop(&mut self) {
> +        // SAFETY: The type invariant guarantees that the pointer is valid.
> +        unsafe {
> +            dealloc(
> +                self.ptr.cast(),
> +                Layout::from_size_align(self.size, 2).unwrap(),
> +            );

Why do we own the pointer (i.e. why can we deallocate the memory)? Add as
a TI (type invariant). Why are you using `dealloc`? Is there no C
function that allocates a `struct shash_desc`? Why is the alignment 2?

> +        }
> +    }
> +}
> +
> +impl<'a> ShashDesc<'a> {
> +    /// Creates a [`ShashDesc`] object for a request data structure for message digest.
> +    pub fn new(tfm: &'a Shash) -> Result<Self> {
> +        // SAFETY: The type invariant guarantees that `tfm.0` pointer is valid.
> +        let size = core::mem::size_of::<bindings::shash_desc>()
> +            + unsafe { bindings::crypto_shash_descsize(tfm.0) } as usize;
> +        let layout = Layout::from_size_align(size, 2)?;
> +        let ptr = unsafe { alloc(layout) } as *mut bindings::shash_desc;

Several things:
- The `SAFETY` comment for `crypto_shash_descsize` should be directly above
  the `unsafe` block,maybe factor that out into its own variable.
- Why is 2 the right alignment?
- Missing `SAFETY` comment for `alloc`.
- Why are you manually creating this layout from size and alignment? Is it
  not possible to do it via the `Layout` API?

> +        if ptr.is_null() {
> +            return Err(ENOMEM);
> +        }
> +        // INVARIANT: `ptr` is valid and non-null since `alloc`
> +        // returned a valid pointer which was null-checked.
> +        let mut desc = ShashDesc { ptr, tfm, size };
> +        // SAFETY: `desc.ptr` is valid and non-null since `alloc`
> +        // returned a valid pointer which was null-checked.
> +        // Additionally, The type invariant guarantees that `tfm.0` is valid.
> +        unsafe { (*desc.ptr).tfm = desc.tfm.0 };
> +        desc.reset()?;
> +        Ok(desc)
> +    }
> +
> +    /// Re-initializes message digest.
> +    pub fn reset(&mut self) -> Result {
> +        // SAFETY: The type invariant guarantees that the pointer is valid.
> +        to_result(unsafe { bindings::crypto_shash_init(self.ptr) })
> +    }
> +
> +    /// Adds data to message digest for processing.
> +    pub fn update(&mut self, data: &[u8]) -> Result {
> +        // SAFETY: The type invariant guarantees that the pointer is valid.
> +        to_result(unsafe {
> +            bindings::crypto_shash_update(self.ptr, data.as_ptr(), data.len() as u32)
> +        })

What if `data.len() > u32::MAX`?

> +    }
> +
> +    /// Calculates message digest.
> +    pub fn finalize(&mut self, output: &mut [u8]) -> Result {
> +        // SAFETY: The type invariant guarantees that the pointer is valid.
> +        to_result(unsafe { bindings::crypto_shash_final(self.ptr, output.as_mut_ptr()) })

As already mentioned by Alex, this needs a size check.

--
Cheers,
Benno

> +    }
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 85b261209977..3cb8bd8a17d9 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -31,6 +31,8 @@
>  #[cfg(not(testlib))]
>  mod allocator;
>  mod build_assert;
> +#[cfg(CONFIG_CRYPTO)]
> +pub mod crypto;
>  pub mod error;
>  pub mod init;
>  pub mod ioctl;
> --
> 2.34.1
> 

Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support

[FUJITA Tomonori]

Hi, thanks a lot for reviewing!

On Mon, 19 Jun 2023 11:40:40 +0000
Benno Lossin <benno.lossin@proton.me> wrote:

>> diff --git a/rust/kernel/crypto/hash.rs b/rust/kernel/crypto/hash.rs
>> new file mode 100644
>> index 000000000000..53f4a311b3b2
>> --- /dev/null
>> +++ b/rust/kernel/crypto/hash.rs
>> @@ -0,0 +1,118 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Cryptographic Hash operations.
>> +//!
>> +//! C headers: [`include/crypto/hash.h`](../../../../include/crypto/hash.h)
>> +
>> +use crate::{
>> +    error::{code::ENOMEM, from_err_ptr, to_result, Result},
>> +    str::CStr,
>> +};
>> +use alloc::alloc::{alloc, dealloc};
>> +use core::alloc::Layout;
>> +
>> +/// Corresponds to the kernel's `struct crypto_shash`.
>> +///
>> +/// # Invariants
>> +///
>> +/// The pointer is valid.
>> +pub struct Shash(*mut bindings::crypto_shash);
>> +
>> +impl Drop for Shash {
>> +    fn drop(&mut self) {
>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>> +        unsafe { bindings::crypto_free_shash(self.0) }
>> +    }
>> +}
>> +
>> +impl Shash {
>> +    /// Creates a [`Shash`] object for a message digest handle.
>> +    pub fn new(name: &CStr, t: u32, mask: u32) -> Result<Shash> {
>> +        // SAFETY: FFI call.
> 
> If there are no requirements then say so.

Understood. Looks like we have variation. Might be better to add the
standard to Documentation/rust/coding-guidelines.rst or such?

ubuntu@ip-172-30-47-114:~/git/linux$ grep -r FFI rust/kernel|grep SAFE
rust/kernel/task.rs:        // SAFETY: Just an FFI call with no additional safety requirements.
rust/kernel/lib.rs:    // SAFETY: FFI call.
rust/kernel/sync/arc.rs:            // SAFETY: There are no safety requirements for this FFI call.
rust/kernel/sync/arc.rs:            // SAFETY: There are no safety requirements for this FFI call.
rust/kernel/error.rs:        // SAFETY: Just an FFI call, there are no extra safety requirements.
rust/kernel/error.rs:///     // SAFETY: FFI call.
rust/kernel/error.rs:    // SAFETY: The FFI function does not deref the pointer.
rust/kernel/error.rs:        // SAFETY: The FFI function does not deref the pointer.

>> +        let ptr =
>> +            unsafe { from_err_ptr(bindings::crypto_alloc_shash(name.as_char_ptr(), t, mask)) }?;
>> +        // INVARIANT: `ptr` is valid and non-null since `crypto_alloc_shash`
>> +        // returned a valid pointer which was null-checked.
>> +        Ok(Self(ptr))
>> +    }
>> +
>> +    /// Sets optional key used by the hashing algorithm.
>> +    pub fn setkey(&mut self, data: &[u8]) -> Result {
> 
> This should be called `set_key`.

I thought that using C function names is a recommended way because
it's easier for subsystem maintainers to review.


>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>> +        to_result(unsafe {
>> +            bindings::crypto_shash_setkey(self.0, data.as_ptr(), data.len() as u32)
>> +        })
>> +    }
>> +
>> +    /// Returns the size of the result of the transformation.
>> +    pub fn digestsize(&self) -> u32 {
> 
> This should be called `digest_size`.

Ditto.

>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>> +        unsafe { bindings::crypto_shash_digestsize(self.0) }
>> +    }
>> +}
>> +
>> +/// Corresponds to the kernel's `struct shash_desc`.
>> +///
>> +/// # Invariants
>> +///
>> +/// The field `ptr` is valid.
>> +pub struct ShashDesc<'a> {
>> +    ptr: *mut bindings::shash_desc,
>> +    tfm: &'a Shash,
>> +    size: usize,
>> +}
>> +
>> +impl Drop for ShashDesc<'_> {
>> +    fn drop(&mut self) {
>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>> +        unsafe {
>> +            dealloc(
>> +                self.ptr.cast(),
>> +                Layout::from_size_align(self.size, 2).unwrap(),
>> +            );
> 
> Why do we own the pointer (i.e. why can we deallocate the memory)? Add as
> a TI (type invariant). Why are you using `dealloc`? Is there no C
> function that allocates a `struct shash_desc`? Why is the alignment 2?

No C function that allocates `struct shash_desc`. kmalloc() family is
used in the C side (or stack is used).

IIUC, the alignment isn't used in the kernel but dealloc() still
requires, right? I'm not sure what number should be used here.

>> +        }
>> +    }
>> +}
>> +
>> +impl<'a> ShashDesc<'a> {
>> +    /// Creates a [`ShashDesc`] object for a request data structure for message digest.
>> +    pub fn new(tfm: &'a Shash) -> Result<Self> {
>> +        // SAFETY: The type invariant guarantees that `tfm.0` pointer is valid.
>> +        let size = core::mem::size_of::<bindings::shash_desc>()
>> +            + unsafe { bindings::crypto_shash_descsize(tfm.0) } as usize;
>> +        let layout = Layout::from_size_align(size, 2)?;
>> +        let ptr = unsafe { alloc(layout) } as *mut bindings::shash_desc;
> 
> Several things:
> - The `SAFETY` comment for `crypto_shash_descsize` should be directly above
>   the `unsafe` block,maybe factor that out into its own variable.

Ok.

> - Why is 2 the right alignment?

As long as the size is larger than alignment, alignment arugment is
meaningless. Like dealloc, not sure what should be used.


> - Missing `SAFETY` comment for `alloc`.

Will be fixed.

> - Why are you manually creating this layout from size and alignment? Is it
>   not possible to do it via the `Layout` API?

What function should be used?

>> +        if ptr.is_null() {
>> +            return Err(ENOMEM);
>> +        }
>> +        // INVARIANT: `ptr` is valid and non-null since `alloc`
>> +        // returned a valid pointer which was null-checked.
>> +        let mut desc = ShashDesc { ptr, tfm, size };
>> +        // SAFETY: `desc.ptr` is valid and non-null since `alloc`
>> +        // returned a valid pointer which was null-checked.
>> +        // Additionally, The type invariant guarantees that `tfm.0` is valid.
>> +        unsafe { (*desc.ptr).tfm = desc.tfm.0 };
>> +        desc.reset()?;
>> +        Ok(desc)
>> +    }
>> +
>> +    /// Re-initializes message digest.
>> +    pub fn reset(&mut self) -> Result {
>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>> +        to_result(unsafe { bindings::crypto_shash_init(self.ptr) })
>> +    }
>> +
>> +    /// Adds data to message digest for processing.
>> +    pub fn update(&mut self, data: &[u8]) -> Result {
>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>> +        to_result(unsafe {
>> +            bindings::crypto_shash_update(self.ptr, data.as_ptr(), data.len() as u32)
>> +        })
> 
> What if `data.len() > u32::MAX`?

The buffer might not be updated properly, I guess. Should check the case?

>> +    }
>> +
>> +    /// Calculates message digest.
>> +    pub fn finalize(&mut self, output: &mut [u8]) -> Result {
>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>> +        to_result(unsafe { bindings::crypto_shash_final(self.ptr, output.as_mut_ptr()) })
> 
> As already mentioned by Alex, this needs a size check.

Will be fixed.


thanks,

Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support

[Benno Lossin]

On 6/22/23 04:14, FUJITA Tomonori wrote:
> Hi, thanks a lot for reviewing!
> 
> On Mon, 19 Jun 2023 11:40:40 +0000
> Benno Lossin <benno.lossin@proton.me> wrote:
> 
>>> diff --git a/rust/kernel/crypto/hash.rs b/rust/kernel/crypto/hash.rs
>>> new file mode 100644
>>> index 000000000000..53f4a311b3b2
>>> --- /dev/null
>>> +++ b/rust/kernel/crypto/hash.rs
>>> @@ -0,0 +1,118 @@
>>> +// SPDX-License-Identifier: GPL-2.0
>>> +
>>> +//! Cryptographic Hash operations.
>>> +//!
>>> +//! C headers: [`include/crypto/hash.h`](../../../../include/crypto/hash.h)
>>> +
>>> +use crate::{
>>> +    error::{code::ENOMEM, from_err_ptr, to_result, Result},
>>> +    str::CStr,
>>> +};
>>> +use alloc::alloc::{alloc, dealloc};
>>> +use core::alloc::Layout;
>>> +
>>> +/// Corresponds to the kernel's `struct crypto_shash`.
>>> +///
>>> +/// # Invariants
>>> +///
>>> +/// The pointer is valid.
>>> +pub struct Shash(*mut bindings::crypto_shash);
>>> +
>>> +impl Drop for Shash {
>>> +    fn drop(&mut self) {
>>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>>> +        unsafe { bindings::crypto_free_shash(self.0) }
>>> +    }
>>> +}
>>> +
>>> +impl Shash {
>>> +    /// Creates a [`Shash`] object for a message digest handle.
>>> +    pub fn new(name: &CStr, t: u32, mask: u32) -> Result<Shash> {
>>> +        // SAFETY: FFI call.
>>
>> If there are no requirements then say so.
> 
> Understood. Looks like we have variation. Might be better to add the
> standard to Documentation/rust/coding-guidelines.rst or such?
> 
> ubuntu@ip-172-30-47-114:~/git/linux$ grep -r FFI rust/kernel|grep SAFE
> rust/kernel/task.rs:        // SAFETY: Just an FFI call with no additional safety requirements.
> rust/kernel/lib.rs:    // SAFETY: FFI call.
> rust/kernel/sync/arc.rs:            // SAFETY: There are no safety requirements for this FFI call.
> rust/kernel/sync/arc.rs:            // SAFETY: There are no safety requirements for this FFI call.
> rust/kernel/error.rs:        // SAFETY: Just an FFI call, there are no extra safety requirements.
> rust/kernel/error.rs:///     // SAFETY: FFI call.
> rust/kernel/error.rs:    // SAFETY: The FFI function does not deref the pointer.
> rust/kernel/error.rs:        // SAFETY: The FFI function does not deref the pointer.

I am currently working on more standardized safety comments. Until we 
agree on how it should be, you can just choose a wording that you think 
is OK.

> 
>>> +        let ptr =
>>> +            unsafe { from_err_ptr(bindings::crypto_alloc_shash(name.as_char_ptr(), t, mask)) }?;
>>> +        // INVARIANT: `ptr` is valid and non-null since `crypto_alloc_shash`
>>> +        // returned a valid pointer which was null-checked.
>>> +        Ok(Self(ptr))
>>> +    }
>>> +
>>> +    /// Sets optional key used by the hashing algorithm.
>>> +    pub fn setkey(&mut self, data: &[u8]) -> Result {
>>
>> This should be called `set_key`.
> 
> I thought that using C function names is a recommended way because
> it's easier for subsystem maintainers to review.

IMO having a `_` that separates words helps a lot with readability. 
Especially with `digestsize`. I also think that adding an `_` will not 
confuse the subsystem maintainers, so we should just do it.

> 
> 
>>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>>> +        to_result(unsafe {
>>> +            bindings::crypto_shash_setkey(self.0, data.as_ptr(), data.len() as u32)
>>> +        })
>>> +    }
>>> +
>>> +    /// Returns the size of the result of the transformation.
>>> +    pub fn digestsize(&self) -> u32 {
>>
>> This should be called `digest_size`.
> 
> Ditto.
> 
>>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>>> +        unsafe { bindings::crypto_shash_digestsize(self.0) }
>>> +    }
>>> +}
>>> +
>>> +/// Corresponds to the kernel's `struct shash_desc`.
>>> +///
>>> +/// # Invariants
>>> +///
>>> +/// The field `ptr` is valid.
>>> +pub struct ShashDesc<'a> {
>>> +    ptr: *mut bindings::shash_desc,
>>> +    tfm: &'a Shash,
>>> +    size: usize,
>>> +}
>>> +
>>> +impl Drop for ShashDesc<'_> {
>>> +    fn drop(&mut self) {
>>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>>> +        unsafe {
>>> +            dealloc(
>>> +                self.ptr.cast(),
>>> +                Layout::from_size_align(self.size, 2).unwrap(),
>>> +            );
>>
>> Why do we own the pointer (i.e. why can we deallocate the memory)? Add as
>> a TI (type invariant). Why are you using `dealloc`? Is there no C
>> function that allocates a `struct shash_desc`? Why is the alignment 2?
> 
> No C function that allocates `struct shash_desc`. kmalloc() family is
> used in the C side (or stack is used).
> 
> IIUC, the alignment isn't used in the kernel but dealloc() still
> requires, right? I'm not sure what number should be used here.

CC'ing Gary, since I am not familiar with `dealloc` in the kernel.
I think the value of the alignment should still be correct if at some 
point in the future `dealloc` starts to use it again.

> 
>>> +        }
>>> +    }
>>> +}
>>> +
>>> +impl<'a> ShashDesc<'a> {
>>> +    /// Creates a [`ShashDesc`] object for a request data structure for message digest.
>>> +    pub fn new(tfm: &'a Shash) -> Result<Self> {
>>> +        // SAFETY: The type invariant guarantees that `tfm.0` pointer is valid.
>>> +        let size = core::mem::size_of::<bindings::shash_desc>()
>>> +            + unsafe { bindings::crypto_shash_descsize(tfm.0) } as usize;
>>> +        let layout = Layout::from_size_align(size, 2)?;
>>> +        let ptr = unsafe { alloc(layout) } as *mut bindings::shash_desc;
>>
>> Several things:
>> - The `SAFETY` comment for `crypto_shash_descsize` should be directly above
>>    the `unsafe` block,maybe factor that out into its own variable.
> 
> Ok.
> 
>> - Why is 2 the right alignment?
> 
> As long as the size is larger than alignment, alignment arugment is
> meaningless. Like dealloc, not sure what should be used.
> 
> 
>> - Missing `SAFETY` comment for `alloc`.
> 
> Will be fixed.
> 
>> - Why are you manually creating this layout from size and alignment? Is it
>>    not possible to do it via the `Layout` API?
> 
> What function should be used?

Maybe `Layout::new()`, `Layout::extend` and `Layout::repeat` might be 
enough?

> 
>>> +        if ptr.is_null() {
>>> +            return Err(ENOMEM);
>>> +        }
>>> +        // INVARIANT: `ptr` is valid and non-null since `alloc`
>>> +        // returned a valid pointer which was null-checked.
>>> +        let mut desc = ShashDesc { ptr, tfm, size };
>>> +        // SAFETY: `desc.ptr` is valid and non-null since `alloc`
>>> +        // returned a valid pointer which was null-checked.
>>> +        // Additionally, The type invariant guarantees that `tfm.0` is valid.
>>> +        unsafe { (*desc.ptr).tfm = desc.tfm.0 };
>>> +        desc.reset()?;
>>> +        Ok(desc)
>>> +    }
>>> +
>>> +    /// Re-initializes message digest.
>>> +    pub fn reset(&mut self) -> Result {
>>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>>> +        to_result(unsafe { bindings::crypto_shash_init(self.ptr) })
>>> +    }
>>> +
>>> +    /// Adds data to message digest for processing.
>>> +    pub fn update(&mut self, data: &[u8]) -> Result {
>>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>>> +        to_result(unsafe {
>>> +            bindings::crypto_shash_update(self.ptr, data.as_ptr(), data.len() as u32)
>>> +        })
>>
>> What if `data.len() > u32::MAX`?
> 
> The buffer might not be updated properly, I guess. Should check the case?

Not sure what we should do in that case, will bring it up at the next 
team meeting. In Rust, `write` and `read` functions often output the 
number of bytes that were actually read/written. So maybe we should also 
do that here? Then you could just return `u32::MAX` and the user would 
have to call again. We could also call the C side multiple times until 
the entire buffer has been processed. But as the C side only supports 
u32 anyway, I think it would be a rare occurrence for `data` to be large.

-- 
Cheers,
Benno

> 
>>> +    }
>>> +
>>> +    /// Calculates message digest.
>>> +    pub fn finalize(&mut self, output: &mut [u8]) -> Result {
>>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>>> +        to_result(unsafe { bindings::crypto_shash_final(self.ptr, output.as_mut_ptr()) })
>>
>> As already mentioned by Alex, this needs a size check.
> 
> Will be fixed.
> 
> 
> thanks,

Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support

[FUJITA Tomonori]

Hi,

On Sun, 25 Jun 2023 10:08:29 +0000
Benno Lossin <benno.lossin@proton.me> wrote:

(snip)

>>>> +        let ptr =
>>>> +            unsafe { from_err_ptr(bindings::crypto_alloc_shash(name.as_char_ptr(), t, mask)) }?;
>>>> +        // INVARIANT: `ptr` is valid and non-null since `crypto_alloc_shash`
>>>> +        // returned a valid pointer which was null-checked.
>>>> +        Ok(Self(ptr))
>>>> +    }
>>>> +
>>>> +    /// Sets optional key used by the hashing algorithm.
>>>> +    pub fn setkey(&mut self, data: &[u8]) -> Result {
>>>
>>> This should be called `set_key`.
>> 
>> I thought that using C function names is a recommended way because
>> it's easier for subsystem maintainers to review.
> 
> IMO having a `_` that separates words helps a lot with readability. 
> Especially with `digestsize`. I also think that adding an `_` will not 
> confuse the subsystem maintainers, so we should just do it.

Looks like `digestsize` is more popular in the tree so let's wait for
reviewing from the crypto maintainers:

ubuntu@ip-172-30-47-114:~/git/linux$ grep -or digestsize crypto/|wc -l
112
ubuntu@ip-172-30-47-114:~/git/linux$ grep -or digest_size crypto/|wc -l
37

>>>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>>>> +        to_result(unsafe {
>>>> +            bindings::crypto_shash_setkey(self.0, data.as_ptr(), data.len() as u32)
>>>> +        })
>>>> +    }
>>>> +
>>>> +    /// Returns the size of the result of the transformation.
>>>> +    pub fn digestsize(&self) -> u32 {
>>>
>>> This should be called `digest_size`.
>> 
>> Ditto.
>> 
>>>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>>>> +        unsafe { bindings::crypto_shash_digestsize(self.0) }
>>>> +    }
>>>> +}
>>>> +
>>>> +/// Corresponds to the kernel's `struct shash_desc`.
>>>> +///
>>>> +/// # Invariants
>>>> +///
>>>> +/// The field `ptr` is valid.
>>>> +pub struct ShashDesc<'a> {
>>>> +    ptr: *mut bindings::shash_desc,
>>>> +    tfm: &'a Shash,
>>>> +    size: usize,
>>>> +}
>>>> +
>>>> +impl Drop for ShashDesc<'_> {
>>>> +    fn drop(&mut self) {
>>>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>>>> +        unsafe {
>>>> +            dealloc(
>>>> +                self.ptr.cast(),
>>>> +                Layout::from_size_align(self.size, 2).unwrap(),
>>>> +            );
>>>
>>> Why do we own the pointer (i.e. why can we deallocate the memory)? Add as
>>> a TI (type invariant). Why are you using `dealloc`? Is there no C
>>> function that allocates a `struct shash_desc`? Why is the alignment 2?
>> 
>> No C function that allocates `struct shash_desc`. kmalloc() family is
>> used in the C side (or stack is used).
>> 
>> IIUC, the alignment isn't used in the kernel but dealloc() still
>> requires, right? I'm not sure what number should be used here.
> 
> CC'ing Gary, since I am not familiar with `dealloc` in the kernel.
> I think the value of the alignment should still be correct if at some 
> point in the future `dealloc` starts to use it again.
> 
>> 
>>>> +        }
>>>> +    }
>>>> +}
>>>> +
>>>> +impl<'a> ShashDesc<'a> {
>>>> +    /// Creates a [`ShashDesc`] object for a request data structure for message digest.
>>>> +    pub fn new(tfm: &'a Shash) -> Result<Self> {
>>>> +        // SAFETY: The type invariant guarantees that `tfm.0` pointer is valid.
>>>> +        let size = core::mem::size_of::<bindings::shash_desc>()
>>>> +            + unsafe { bindings::crypto_shash_descsize(tfm.0) } as usize;
>>>> +        let layout = Layout::from_size_align(size, 2)?;
>>>> +        let ptr = unsafe { alloc(layout) } as *mut bindings::shash_desc;
>>>
>>> Several things:
>>> - The `SAFETY` comment for `crypto_shash_descsize` should be directly above
>>>    the `unsafe` block,maybe factor that out into its own variable.
>> 
>> Ok.
>> 
>>> - Why is 2 the right alignment?
>> 
>> As long as the size is larger than alignment, alignment arugment is
>> meaningless. Like dealloc, not sure what should be used.
>> 
>> 
>>> - Missing `SAFETY` comment for `alloc`.
>> 
>> Will be fixed.
>> 
>>> - Why are you manually creating this layout from size and alignment? Is it
>>>    not possible to do it via the `Layout` API?
>> 
>> What function should be used?
> 
> Maybe `Layout::new()`, `Layout::extend` and `Layout::repeat` might be 
> enough?

new() needs type and extend() and repeat() need self; both is
irrelevant here.

>>>> +        if ptr.is_null() {
>>>> +            return Err(ENOMEM);
>>>> +        }
>>>> +        // INVARIANT: `ptr` is valid and non-null since `alloc`
>>>> +        // returned a valid pointer which was null-checked.
>>>> +        let mut desc = ShashDesc { ptr, tfm, size };
>>>> +        // SAFETY: `desc.ptr` is valid and non-null since `alloc`
>>>> +        // returned a valid pointer which was null-checked.
>>>> +        // Additionally, The type invariant guarantees that `tfm.0` is valid.
>>>> +        unsafe { (*desc.ptr).tfm = desc.tfm.0 };
>>>> +        desc.reset()?;
>>>> +        Ok(desc)
>>>> +    }
>>>> +
>>>> +    /// Re-initializes message digest.
>>>> +    pub fn reset(&mut self) -> Result {
>>>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>>>> +        to_result(unsafe { bindings::crypto_shash_init(self.ptr) })
>>>> +    }
>>>> +
>>>> +    /// Adds data to message digest for processing.
>>>> +    pub fn update(&mut self, data: &[u8]) -> Result {
>>>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>>>> +        to_result(unsafe {
>>>> +            bindings::crypto_shash_update(self.ptr, data.as_ptr(), data.len() as u32)
>>>> +        })
>>>
>>> What if `data.len() > u32::MAX`?
>> 
>> The buffer might not be updated properly, I guess. Should check the case?
> 
> Not sure what we should do in that case, will bring it up at the next 
> team meeting. In Rust, `write` and `read` functions often output the 
> number of bytes that were actually read/written. So maybe we should also 
> do that here? Then you could just return `u32::MAX` and the user would 
> have to call again. We could also call the C side multiple times until 
> the entire buffer has been processed. But as the C side only supports 
> u32 anyway, I think it would be a rare occurrence for `data` to be large.

I'll change the code to return an error in this case. I prefer not to
extend C logic (like calling a C function multiple times) but if there
is an official policy for Rust bindings, I'll change the code to
follow the policy.


thanks,

Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support

[Benno Lossin]

Dear crypto maintainers,

Fujita Tomonori has created some Rust bindings for the crypto API seen in
this thread. Here is a fragment of my review of said code:

On 25.06.23 12:08, Benno Lossin wrote:
>>>> +    /// Adds data to message digest for processing.
>>>> +    pub fn update(&mut self, data: &[u8]) -> Result {
>>>> +        // SAFETY: The type invariant guarantees that the pointer is valid.
>>>> +        to_result(unsafe {
>>>> +            bindings::crypto_shash_update(self.ptr, data.as_ptr(), data.len() as u32)
>>>> +        })
>>>
>>> What if `data.len() > u32::MAX`?
>>
>> The buffer might not be updated properly, I guess. Should check the case?
> 
> Not sure what we should do in that case, will bring it up at the next
> team meeting. In Rust, `write` and `read` functions often output the
> number of bytes that were actually read/written. So maybe we should also
> do that here? Then you could just return `u32::MAX` and the user would
> have to call again. We could also call the C side multiple times until
> the entire buffer has been processed. But as the C side only supports
> u32 anyway, I think it would be a rare occurrence for `data` to be large.

I noted that in the code segment above that the length of the data
that is to be hashed is cast from a `usize` to a `u32`. Since
`usize = uintptr_t` this might be a problem for very large arguments.

Since the C side only accepts an `unsigned int`, it seems as if large inputs
are never the case. On the Rust side we are forced to use `usize`, since that
is the length of slices (the input type `&[u8]`).

We came up with the following solutions, but could not come to a consensus on any
particular one, could you please assist us in making this decision?

1. create a loop that calls the C API multiple times if the input is large
2. panic
3. truncate
4. return an error

Thanks a lot!

--
Cheers,
Benno

Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support

[Greg KH]

On Fri, Jun 30, 2023 at 02:48:37PM +0000, Benno Lossin wrote:
> We came up with the following solutions, but could not come to a consensus on any
> particular one, could you please assist us in making this decision?
> 
> 2. panic

Never an option in kernel code, sorry, please don't even suggest it.

thanks,

greg k-h

Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support

[Herbert Xu]

On Fri, Jun 30, 2023 at 02:48:37PM +0000, Benno Lossin wrote:
>
> 4. return an error

This would seem to make the most sense.

If there is ever a need to hash more than 4G of data, we would
be adding this to C first.

At this point I can't see why we would need to do that so an
error would be the appropriate response.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support

[Benno Lossin]

------- Original Message -------
On Tuesday, July 4th, 2023 at 01:19, Herbert Xu <herbert@gondor.apana.org.au> wrote:
> On Fri, Jun 30, 2023 at 02:48:37PM +0000, Benno Lossin wrote:
> 
> > 4. return an error
> 
> 
> This would seem to make the most sense.
> 
> If there is ever a need to hash more than 4G of data, we would
> be adding this to C first.
> 
> At this point I can't see why we would need to do that so an
> error would be the appropriate response.
> 
> Thanks,
> --
> Email: Herbert Xu herbert@gondor.apana.org.au
> 
> Home Page: http://gondor.apana.org.au/~herbert/
> PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Thanks a lot for taking a look!

--
Cheers,
Benno

[RFC PATCH v2 2/2] rust: add Random Number Generator algorithms support

[FUJITA Tomonori]

Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
---
 rust/bindings/bindings_helper.h |  1 +
 rust/helpers.c                  | 12 ++++++
 rust/kernel/crypto.rs           |  1 +
 rust/kernel/crypto/rng.rs       | 72 +++++++++++++++++++++++++++++++++
 4 files changed, 86 insertions(+)
 create mode 100644 rust/kernel/crypto/rng.rs

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 2f198c6d5de5..089ac38c6461 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -7,6 +7,7 @@
  */
 
 #include <crypto/hash.h>
+#include <crypto/rng.h>
 #include <linux/errname.h>
 #include <linux/slab.h>
 #include <linux/refcount.h>
diff --git a/rust/helpers.c b/rust/helpers.c
index 7966902ed8eb..e4dcd611738f 100644
--- a/rust/helpers.c
+++ b/rust/helpers.c
@@ -19,6 +19,7 @@
  */
 
 #include <crypto/hash.h>
+#include <crypto/rng.h>
 #include <linux/bug.h>
 #include <linux/build_bug.h>
 #include <linux/err.h>
@@ -52,6 +53,17 @@ int rust_helper_crypto_shash_init(struct shash_desc *desc) {
 	return crypto_shash_init(desc);
 }
 EXPORT_SYMBOL_GPL(rust_helper_crypto_shash_init);
+
+void rust_helper_crypto_free_rng(struct crypto_rng *tfm) {
+	crypto_free_rng(tfm);
+}
+EXPORT_SYMBOL_GPL(rust_helper_crypto_free_rng);
+
+int rust_helper_crypto_rng_generate(struct crypto_rng *tfm, const u8 *src,
+	unsigned int slen, u8 *dst, unsigned int dlen) {
+	return crypto_rng_generate(tfm, src, slen, dst, dlen);
+}
+EXPORT_SYMBOL_GPL(rust_helper_crypto_rng_generate);
 #endif
 
 __noreturn void rust_helper_BUG(void)
diff --git a/rust/kernel/crypto.rs b/rust/kernel/crypto.rs
index f80dd7bd3381..a1995e6c85d4 100644
--- a/rust/kernel/crypto.rs
+++ b/rust/kernel/crypto.rs
@@ -3,3 +3,4 @@
 //! Cryptography.
 
 pub mod hash;
+pub mod rng;
diff --git a/rust/kernel/crypto/rng.rs b/rust/kernel/crypto/rng.rs
new file mode 100644
index 000000000000..2215a8344669
--- /dev/null
+++ b/rust/kernel/crypto/rng.rs
@@ -0,0 +1,72 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Random number generator.
+//!
+//! C headers: [`include/crypto/rng.h`](../../../../include/crypto/rng.h)
+
+use crate::{
+    error::{from_err_ptr, to_result, Result},
+    str::CStr,
+};
+
+/// Corresponds to the kernel's `struct crypto_rng`.
+///
+/// # Invariants
+///
+/// The pointer is valid.
+pub struct Rng(*mut bindings::crypto_rng);
+
+impl Drop for Rng {
+    fn drop(&mut self) {
+        // SAFETY: The type invariant guarantees that `self.0` is valid.
+        if unsafe { bindings::crypto_default_rng } == self.0 {
+            // SAFETY: FFI call.
+            unsafe {
+                bindings::crypto_put_default_rng();
+            }
+        } else {
+            // SAFETY: The type invariant guarantees that `self.0` is valid.
+            unsafe { bindings::crypto_free_rng(self.0) };
+        }
+    }
+}
+
+impl Rng {
+    /// Creates a [`Rng`] instance.
+    pub fn new(name: &CStr, t: u32, mask: u32) -> Result<Self> {
+        // SAFETY: FFI call.
+        let ptr = unsafe { from_err_ptr(bindings::crypto_alloc_rng(name.as_char_ptr(), t, mask)) }?;
+        // INVARIANT: `ptr` is valid and non-null since `crypto_alloc_rng`
+        // returned a valid pointer which was null-checked.
+        Ok(Self(ptr))
+    }
+
+    /// Creates a [`Rng`] instance with a default algorithm.
+    pub fn new_with_default() -> Result<Self> {
+        // SAFETY: FFI call.
+        to_result(unsafe { bindings::crypto_get_default_rng() })?;
+        // SAFETY: The C API guarantees that `crypto_default_rng` is valid until
+        // `crypto_put_default_rng` is called.
+        Ok(Self(unsafe { bindings::crypto_default_rng }))
+    }
+
+    /// Get a random number.
+    pub fn generate(&mut self, src: &[u8], dst: &mut [u8]) -> Result {
+        // SAFETY: The type invariant guarantees that the pointer is valid.
+        to_result(unsafe {
+            bindings::crypto_rng_generate(
+                self.0,
+                src.as_ptr(),
+                src.len() as u32,
+                dst.as_mut_ptr(),
+                dst.len() as u32,
+            )
+        })
+    }
+
+    /// Re-initializes the [`Rng`] instance.
+    pub fn reset(&mut self, seed: &[u8]) -> Result {
+        // SAFETY: The type invariant guarantees that the pointer is valid.
+        to_result(unsafe { bindings::crypto_rng_reset(self.0, seed.as_ptr(), seed.len() as u32) })
+    }
+}
-- 
2.34.1

Re: [RFC PATCH v2 2/2] rust: add Random Number Generator algorithms support

[Benno Lossin]

> Signed-off-by: FUJITA Tomonori <fujita.tomonori@gmail.com>
> ---
>  rust/bindings/bindings_helper.h |  1 +
>  rust/helpers.c                  | 12 ++++++
>  rust/kernel/crypto.rs           |  1 +
>  rust/kernel/crypto/rng.rs       | 72 +++++++++++++++++++++++++++++++++
>  4 files changed, 86 insertions(+)
>  create mode 100644 rust/kernel/crypto/rng.rs
> 
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 2f198c6d5de5..089ac38c6461 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -7,6 +7,7 @@
>   */
> 
>  #include <crypto/hash.h>
> +#include <crypto/rng.h>
>  #include <linux/errname.h>
>  #include <linux/slab.h>
>  #include <linux/refcount.h>
> diff --git a/rust/helpers.c b/rust/helpers.c
> index 7966902ed8eb..e4dcd611738f 100644
> --- a/rust/helpers.c
> +++ b/rust/helpers.c
> @@ -19,6 +19,7 @@
>   */
> 
>  #include <crypto/hash.h>
> +#include <crypto/rng.h>
>  #include <linux/bug.h>
>  #include <linux/build_bug.h>
>  #include <linux/err.h>
> @@ -52,6 +53,17 @@ int rust_helper_crypto_shash_init(struct shash_desc *desc) {
>  	return crypto_shash_init(desc);
>  }
>  EXPORT_SYMBOL_GPL(rust_helper_crypto_shash_init);
> +
> +void rust_helper_crypto_free_rng(struct crypto_rng *tfm) {
> +	crypto_free_rng(tfm);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_crypto_free_rng);
> +
> +int rust_helper_crypto_rng_generate(struct crypto_rng *tfm, const u8 *src,
> +	unsigned int slen, u8 *dst, unsigned int dlen) {
> +	return crypto_rng_generate(tfm, src, slen, dst, dlen);
> +}
> +EXPORT_SYMBOL_GPL(rust_helper_crypto_rng_generate);
>  #endif
> 
>  __noreturn void rust_helper_BUG(void)
> diff --git a/rust/kernel/crypto.rs b/rust/kernel/crypto.rs
> index f80dd7bd3381..a1995e6c85d4 100644
> --- a/rust/kernel/crypto.rs
> +++ b/rust/kernel/crypto.rs
> @@ -3,3 +3,4 @@
>  //! Cryptography.
> 
>  pub mod hash;
> +pub mod rng;
> diff --git a/rust/kernel/crypto/rng.rs b/rust/kernel/crypto/rng.rs
> new file mode 100644
> index 000000000000..2215a8344669
> --- /dev/null
> +++ b/rust/kernel/crypto/rng.rs
> @@ -0,0 +1,72 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Random number generator.
> +//!
> +//! C headers: [`include/crypto/rng.h`](../../../../include/crypto/rng.h)
> +
> +use crate::{
> +    error::{from_err_ptr, to_result, Result},
> +    str::CStr,
> +};
> +
> +/// Corresponds to the kernel's `struct crypto_rng`.
> +///
> +/// # Invariants
> +///
> +/// The pointer is valid.
> +pub struct Rng(*mut bindings::crypto_rng);
> +
> +impl Drop for Rng {
> +    fn drop(&mut self) {
> +        // SAFETY: The type invariant guarantees that `self.0` is valid.

This safety comment should explain why it is safe to access this mutable
static variable.

> +        if unsafe { bindings::crypto_default_rng } == self.0 {
> +            // SAFETY: FFI call.
> +            unsafe {
> +                bindings::crypto_put_default_rng();
> +            }
> +        } else {
> +            // SAFETY: The type invariant guarantees that `self.0` is valid.
> +            unsafe { bindings::crypto_free_rng(self.0) };
> +        }
> +    }
> +}
> +
> +impl Rng {
> +    /// Creates a [`Rng`] instance.
> +    pub fn new(name: &CStr, t: u32, mask: u32) -> Result<Self> {
> +        // SAFETY: FFI call.

If there are no requirements then say so.

> +        let ptr = unsafe { from_err_ptr(bindings::crypto_alloc_rng(name.as_char_ptr(), t, mask)) }?;
> +        // INVARIANT: `ptr` is valid and non-null since `crypto_alloc_rng`
> +        // returned a valid pointer which was null-checked.
> +        Ok(Self(ptr))
> +    }
> +
> +    /// Creates a [`Rng`] instance with a default algorithm.
> +    pub fn new_with_default() -> Result<Self> {
> +        // SAFETY: FFI call.
> +        to_result(unsafe { bindings::crypto_get_default_rng() })?;
> +        // SAFETY: The C API guarantees that `crypto_default_rng` is valid until
> +        // `crypto_put_default_rng` is called.
> +        Ok(Self(unsafe { bindings::crypto_default_rng }))
> +    }
> +
> +    /// Get a random number.
> +    pub fn generate(&mut self, src: &[u8], dst: &mut [u8]) -> Result {
> +        // SAFETY: The type invariant guarantees that the pointer is valid.
> +        to_result(unsafe {
> +            bindings::crypto_rng_generate(
> +                self.0,
> +                src.as_ptr(),
> +                src.len() as u32,

What happens when `src.len() > u32::MAX`? Then it will overflow here and
write less than `len` random bytes. Not sure what we want to do in this 
case though, maybe call it in a loop? Return an error? Also below.

> +                dst.as_mut_ptr(),
> +                dst.len() as u32,
> +            )
> +        })
> +    }
> +
> +    /// Re-initializes the [`Rng`] instance.
> +    pub fn reset(&mut self, seed: &[u8]) -> Result {
> +        // SAFETY: The type invariant guarantees that the pointer is valid.
> +        to_result(unsafe { bindings::crypto_rng_reset(self.0, seed.as_ptr(), seed.len() as u32) })

Same problem with the as u32 cast here.

--
Cheers,
Benno

> +    }
> +}
> --
> 2.34.1
>

Re: [RFC PATCH v2 2/2] rust: add Random Number Generator algorithms support

[FUJITA Tomonori]

Hi,

On Mon, 19 Jun 2023 11:41:29 +0000
Benno Lossin <benno.lossin@proton.me> wrote:

>> diff --git a/rust/kernel/crypto/rng.rs b/rust/kernel/crypto/rng.rs
>> new file mode 100644
>> index 000000000000..2215a8344669
>> --- /dev/null
>> +++ b/rust/kernel/crypto/rng.rs
>> @@ -0,0 +1,72 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Random number generator.
>> +//!
>> +//! C headers: [`include/crypto/rng.h`](../../../../include/crypto/rng.h)
>> +
>> +use crate::{
>> +    error::{from_err_ptr, to_result, Result},
>> +    str::CStr,
>> +};
>> +
>> +/// Corresponds to the kernel's `struct crypto_rng`.
>> +///
>> +/// # Invariants
>> +///
>> +/// The pointer is valid.
>> +pub struct Rng(*mut bindings::crypto_rng);
>> +
>> +impl Drop for Rng {
>> +    fn drop(&mut self) {
>> +        // SAFETY: The type invariant guarantees that `self.0` is valid.
> 
> This safety comment should explain why it is safe to access this mutable
> static variable.
> 
>> +        if unsafe { bindings::crypto_default_rng } == self.0 {
>> +            // SAFETY: FFI call.
>> +            unsafe {
>> +                bindings::crypto_put_default_rng();
>> +            }
>> +        } else {
>> +            // SAFETY: The type invariant guarantees that `self.0` is valid.
>> +            unsafe { bindings::crypto_free_rng(self.0) };
>> +        }
>> +    }
>> +}

crypto_get_default_rng() is called during the initialization so it's
safe to access to bindings::crypto_default_rng. However, it's cleaner
to use an internal type like the following?

diff --git a/rust/kernel/crypto/rng.rs b/rust/kernel/crypto/rng.rs
new file mode 100644
index 000000000000..1d5d16577f3e
--- /dev/null
+++ b/rust/kernel/crypto/rng.rs
@@ -0,0 +1,100 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Random number generator.
+//!
+//! C headers: [`include/crypto/rng.h`](../../../../include/crypto/rng.h)
+
+use crate::{
+    error::{code::EINVAL, from_err_ptr, to_result, Result},
+    str::CStr,
+};
+
+/// Type of Random number generator.
+///
+/// # Invariants
+///
+/// The pointer is valid.
+enum RngType {
+    /// Uses `crypto_default_rng`
+    Default(*mut bindings::crypto_rng),
+
+    /// Allocated via `crypto_alloc_rng.
+    Allocated(*mut bindings::crypto_rng),
+}
+
+/// Corresponds to the kernel's `struct crypto_rng`.
+pub struct Rng(RngType);
+
+impl Drop for Rng {
+    fn drop(&mut self) {
+        match self.0 {
+            RngType::Default(_) => {
+                // SAFETY: it's safe because `crypto_get_default_rng()` was called during
+                // the initialization.
+                unsafe {
+                    bindings::crypto_put_default_rng();
+                }
+            }
+            RngType::Allocated(ptr) => {
+                // SAFETY: The type invariants of `RngType` guarantees that the pointer is valid.
+                unsafe { bindings::crypto_free_rng(ptr) };
+            }
+        }
+    }
+}
+
+impl Rng {
+    /// Creates a [`Rng`] instance.
+    pub fn new(name: &CStr, t: u32, mask: u32) -> Result<Self> {
+        // SAFETY: There are no safety requirements for this FFI call.
+        let ptr = unsafe { from_err_ptr(bindings::crypto_alloc_rng(name.as_char_ptr(), t, mask)) }?;
+        // INVARIANT: `ptr` is valid and non-null since `crypto_alloc_rng`
+        // returned a valid pointer which was null-checked.
+        Ok(Self(RngType::Allocated(ptr)))
+    }
+
+    /// Creates a [`Rng`] instance with a default algorithm.
+    pub fn new_with_default() -> Result<Self> {
+        // SAFETY: There are no safety requirements for this FFI call.
+        to_result(unsafe { bindings::crypto_get_default_rng() })?;
+        // INVARIANT: The C API guarantees that `crypto_default_rng` is valid until
+        // `crypto_put_default_rng` is called.
+        Ok(Self(RngType::Default(unsafe {
+            bindings::crypto_default_rng
+        })))
+    }
+
+    /// Get a random number.
+    pub fn generate(&mut self, src: &[u8], dst: &mut [u8]) -> Result {
+        if src.len() > u32::MAX as usize || dst.len() > u32::MAX as usize {
+            return Err(EINVAL);
+        }
+        let ptr = match self.0 {
+            RngType::Default(ptr) => ptr,
+            RngType::Allocated(ptr) => ptr,
+        };
+        // SAFETY: The type invariants of `RngType' guarantees that the pointer is valid.
+        to_result(unsafe {
+            bindings::crypto_rng_generate(
+                ptr,
+                src.as_ptr(),
+                src.len() as u32,
+                dst.as_mut_ptr(),
+                dst.len() as u32,
+            )
+        })
+    }
+
+    /// Re-initializes the [`Rng`] instance.
+    pub fn reset(&mut self, seed: &[u8]) -> Result {
+        if seed.len() > u32::MAX as usize {
+            return Err(EINVAL);
+        }
+        let ptr = match self.0 {
+            RngType::Default(ptr) => ptr,
+            RngType::Allocated(ptr) => ptr,
+        };
+        // SAFETY: The type invariants of `RngType' guarantees that the pointer is valid.
+        to_result(unsafe { bindings::crypto_rng_reset(ptr, seed.as_ptr(), seed.len() as u32) })
+    }
+}
-- 
2.34.1