* [RFC PATCH v2 0/2] Rust abstractions for Crypto API @ 2023-06-15 14:23 FUJITA Tomonori 2023-06-15 14:23 ` [RFC PATCH v2 1/2] rust: add synchronous message digest support FUJITA Tomonori 2023-06-15 14:23 ` [RFC PATCH v2 2/2] rust: add Random Number Generator algorithms support FUJITA Tomonori 0 siblings, 2 replies; 17+ messages in thread From: FUJITA Tomonori @ 2023-06-15 14:23 UTC (permalink / raw) To: rust-for-linux 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH v2 1/2] rust: add synchronous message digest support 2023-06-15 14:23 [RFC PATCH v2 0/2] Rust abstractions for Crypto API FUJITA Tomonori @ 2023-06-15 14:23 ` FUJITA Tomonori 2023-06-15 15:01 ` Greg KH ` (2 more replies) 2023-06-15 14:23 ` [RFC PATCH v2 2/2] rust: add Random Number Generator algorithms support FUJITA Tomonori 1 sibling, 3 replies; 17+ messages in thread From: FUJITA Tomonori @ 2023-06-15 14:23 UTC (permalink / raw) To: rust-for-linux 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support 2023-06-15 14:23 ` [RFC PATCH v2 1/2] rust: add synchronous message digest support FUJITA Tomonori @ 2023-06-15 15:01 ` Greg KH 2023-06-15 15:33 ` FUJITA Tomonori 2023-06-15 15:02 ` Alex Gaynor 2023-06-19 11:40 ` Benno Lossin 2 siblings, 1 reply; 17+ messages in thread From: Greg KH @ 2023-06-15 15:01 UTC (permalink / raw) To: FUJITA Tomonori; +Cc: rust-for-linux 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support 2023-06-15 15:01 ` Greg KH @ 2023-06-15 15:33 ` FUJITA Tomonori 0 siblings, 0 replies; 17+ messages in thread From: FUJITA Tomonori @ 2023-06-15 15:33 UTC (permalink / raw) To: gregkh; +Cc: fujita.tomonori, rust-for-linux 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, ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support 2023-06-15 14:23 ` [RFC PATCH v2 1/2] rust: add synchronous message digest support FUJITA Tomonori 2023-06-15 15:01 ` Greg KH @ 2023-06-15 15:02 ` Alex Gaynor 2023-06-15 15:24 ` FUJITA Tomonori 2023-06-19 11:40 ` Benno Lossin 2 siblings, 1 reply; 17+ messages in thread From: Alex Gaynor @ 2023-06-15 15:02 UTC (permalink / raw) To: FUJITA Tomonori; +Cc: rust-for-linux 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. ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support 2023-06-15 15:02 ` Alex Gaynor @ 2023-06-15 15:24 ` FUJITA Tomonori 0 siblings, 0 replies; 17+ messages in thread From: FUJITA Tomonori @ 2023-06-15 15:24 UTC (permalink / raw) To: alex.gaynor; +Cc: fujita.tomonori, rust-for-linux 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! ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support 2023-06-15 14:23 ` [RFC PATCH v2 1/2] rust: add synchronous message digest support FUJITA Tomonori 2023-06-15 15:01 ` Greg KH 2023-06-15 15:02 ` Alex Gaynor @ 2023-06-19 11:40 ` Benno Lossin 2023-06-22 2:14 ` FUJITA Tomonori 2 siblings, 1 reply; 17+ messages in thread From: Benno Lossin @ 2023-06-19 11:40 UTC (permalink / raw) To: FUJITA Tomonori; +Cc: rust-for-linux > 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 > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support 2023-06-19 11:40 ` Benno Lossin @ 2023-06-22 2:14 ` FUJITA Tomonori 2023-06-25 10:08 ` Benno Lossin 0 siblings, 1 reply; 17+ messages in thread From: FUJITA Tomonori @ 2023-06-22 2:14 UTC (permalink / raw) To: benno.lossin; +Cc: fujita.tomonori, rust-for-linux 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, ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support 2023-06-22 2:14 ` FUJITA Tomonori @ 2023-06-25 10:08 ` Benno Lossin 2023-06-25 11:55 ` FUJITA Tomonori 2023-06-30 14:48 ` Benno Lossin 0 siblings, 2 replies; 17+ messages in thread From: Benno Lossin @ 2023-06-25 10:08 UTC (permalink / raw) To: FUJITA Tomonori; +Cc: rust-for-linux, Gary Guo 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, ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support 2023-06-25 10:08 ` Benno Lossin @ 2023-06-25 11:55 ` FUJITA Tomonori 2023-06-30 14:48 ` Benno Lossin 1 sibling, 0 replies; 17+ messages in thread From: FUJITA Tomonori @ 2023-06-25 11:55 UTC (permalink / raw) To: benno.lossin; +Cc: fujita.tomonori, rust-for-linux, gary 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, ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support 2023-06-25 10:08 ` Benno Lossin 2023-06-25 11:55 ` FUJITA Tomonori @ 2023-06-30 14:48 ` Benno Lossin 2023-06-30 19:50 ` Greg KH 2023-07-03 23:19 ` Herbert Xu 1 sibling, 2 replies; 17+ messages in thread From: Benno Lossin @ 2023-06-30 14:48 UTC (permalink / raw) To: Herbert Xu, David S. Miller Cc: FUJITA Tomonori, rust-for-linux, Gary Guo, linux-crypto 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support 2023-06-30 14:48 ` Benno Lossin @ 2023-06-30 19:50 ` Greg KH 2023-07-03 23:19 ` Herbert Xu 1 sibling, 0 replies; 17+ messages in thread From: Greg KH @ 2023-06-30 19:50 UTC (permalink / raw) To: Benno Lossin Cc: Herbert Xu, David S. Miller, FUJITA Tomonori, rust-for-linux, Gary Guo, linux-crypto 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support 2023-06-30 14:48 ` Benno Lossin 2023-06-30 19:50 ` Greg KH @ 2023-07-03 23:19 ` Herbert Xu 2023-07-10 19:59 ` Benno Lossin 1 sibling, 1 reply; 17+ messages in thread From: Herbert Xu @ 2023-07-03 23:19 UTC (permalink / raw) To: Benno Lossin Cc: David S. Miller, FUJITA Tomonori, rust-for-linux, Gary Guo, linux-crypto 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 1/2] rust: add synchronous message digest support 2023-07-03 23:19 ` Herbert Xu @ 2023-07-10 19:59 ` Benno Lossin 0 siblings, 0 replies; 17+ messages in thread From: Benno Lossin @ 2023-07-10 19:59 UTC (permalink / raw) To: Herbert Xu Cc: David S. Miller, FUJITA Tomonori, rust-for-linux, Gary Guo, linux-crypto ------- 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 ^ permalink raw reply [flat|nested] 17+ messages in thread
* [RFC PATCH v2 2/2] rust: add Random Number Generator algorithms support 2023-06-15 14:23 [RFC PATCH v2 0/2] Rust abstractions for Crypto API FUJITA Tomonori 2023-06-15 14:23 ` [RFC PATCH v2 1/2] rust: add synchronous message digest support FUJITA Tomonori @ 2023-06-15 14:23 ` FUJITA Tomonori 2023-06-19 11:41 ` Benno Lossin 1 sibling, 1 reply; 17+ messages in thread From: FUJITA Tomonori @ 2023-06-15 14:23 UTC (permalink / raw) To: rust-for-linux 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 2/2] rust: add Random Number Generator algorithms support 2023-06-15 14:23 ` [RFC PATCH v2 2/2] rust: add Random Number Generator algorithms support FUJITA Tomonori @ 2023-06-19 11:41 ` Benno Lossin 2023-07-06 4:05 ` FUJITA Tomonori 0 siblings, 1 reply; 17+ messages in thread From: Benno Lossin @ 2023-06-19 11:41 UTC (permalink / raw) To: FUJITA Tomonori; +Cc: rust-for-linux > 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 > ^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [RFC PATCH v2 2/2] rust: add Random Number Generator algorithms support 2023-06-19 11:41 ` Benno Lossin @ 2023-07-06 4:05 ` FUJITA Tomonori 0 siblings, 0 replies; 17+ messages in thread From: FUJITA Tomonori @ 2023-07-06 4:05 UTC (permalink / raw) To: benno.lossin; +Cc: fujita.tomonori, rust-for-linux 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 ^ permalink raw reply related [flat|nested] 17+ messages in thread
end of thread, other threads:[~2023-07-10 19:59 UTC | newest] Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-06-15 14:23 [RFC PATCH v2 0/2] Rust abstractions for Crypto API FUJITA Tomonori 2023-06-15 14:23 ` [RFC PATCH v2 1/2] rust: add synchronous message digest support FUJITA Tomonori 2023-06-15 15:01 ` Greg KH 2023-06-15 15:33 ` FUJITA Tomonori 2023-06-15 15:02 ` Alex Gaynor 2023-06-15 15:24 ` FUJITA Tomonori 2023-06-19 11:40 ` Benno Lossin 2023-06-22 2:14 ` FUJITA Tomonori 2023-06-25 10:08 ` Benno Lossin 2023-06-25 11:55 ` FUJITA Tomonori 2023-06-30 14:48 ` Benno Lossin 2023-06-30 19:50 ` Greg KH 2023-07-03 23:19 ` Herbert Xu 2023-07-10 19:59 ` Benno Lossin 2023-06-15 14:23 ` [RFC PATCH v2 2/2] rust: add Random Number Generator algorithms support FUJITA Tomonori 2023-06-19 11:41 ` Benno Lossin 2023-07-06 4:05 ` FUJITA Tomonori
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.