| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250326201230.3193329-3-abdiel.janulgue@gmail.com>
Date: Wed, 26 Mar 2025 22:11:43 +0200
From: Abdiel Janulgue <abdiel.janulgue@...il.com>
To: a.hindborg@...nel.org,
ojeda@...nel.org
Cc: Danilo Krummrich <dakr@...nel.org>,
Daniel Almeida <daniel.almeida@...labora.com>,
Robin Murphy <robin.murphy@....com>,
Alex Gaynor <alex.gaynor@...il.com>,
Boqun Feng <boqun.feng@...il.com>,
Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <benno.lossin@...ton.me>,
Alice Ryhl <aliceryhl@...gle.com>,
Trevor Gross <tmgross@...ch.edu>,
rust-for-linux@...r.kernel.org (open list:DMA MAPPING HELPERS DEVICE DRIVER API [RUST]),
Marek Szyprowski <m.szyprowski@...sung.com>,
iommu@...ts.linux.dev (open list:DMA MAPPING HELPERS),
linux-kernel@...r.kernel.org (open list),
Abdiel Janulgue <abdiel.janulgue@...il.com>
Subject: [PATCH 2/3] rust: dma: convert the read/write macros to return Result
As suggested by Andreas Hindborg, we could do better here by
having the macros return `Result`, so that we don't have to wrap
these calls in a closure for validation which is confusing.
Signed-off-by: Abdiel Janulgue <abdiel.janulgue@...il.com>
---
rust/kernel/dma.rs | 54 +++++++++++++++++++++++-----------------
samples/rust/rust_dma.rs | 21 ++++++----------
2 files changed, 38 insertions(+), 37 deletions(-)
diff --git a/rust/kernel/dma.rs b/rust/kernel/dma.rs
index d3f448868457..24a6f10370c4 100644
--- a/rust/kernel/dma.rs
+++ b/rust/kernel/dma.rs
@@ -328,20 +328,22 @@ unsafe impl<T: AsBytes + FromBytes + Send> Send for CoherentAllocation<T> {}
#[macro_export]
macro_rules! dma_read {
($dma:expr, $idx: expr, $($field:tt)*) => {{
- let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?;
- // SAFETY: `item_from_index` ensures that `item` is always a valid pointer and can be
- // dereferenced. The compiler also further validates the expression on whether `field`
- // is a member of `item` when expanded by the macro.
- unsafe {
- let ptr_field = ::core::ptr::addr_of!((*item) $($field)*);
- $crate::dma::CoherentAllocation::field_read(&$dma, ptr_field)
- }
+ (|| -> Result<_> {
+ let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?;
+ // SAFETY: `item_from_index` ensures that `item` is always a valid pointer and can be
+ // dereferenced. The compiler also further validates the expression on whether `field`
+ // is a member of `item` when expanded by the macro.
+ unsafe {
+ let ptr_field = ::core::ptr::addr_of!((*item) $($field)*);
+ ::core::result::Result::Ok($crate::dma::CoherentAllocation::field_read(&$dma, ptr_field))
+ }
+ })()
}};
($dma:ident [ $idx:expr ] $($field:tt)* ) => {
- $crate::dma_read!($dma, $idx, $($field)*);
+ $crate::dma_read!($dma, $idx, $($field)*)
};
($($dma:ident).* [ $idx:expr ] $($field:tt)* ) => {
- $crate::dma_read!($($dma).*, $idx, $($field)*);
+ $crate::dma_read!($($dma).*, $idx, $($field)*)
};
}
@@ -368,24 +370,30 @@ macro_rules! dma_read {
#[macro_export]
macro_rules! dma_write {
($dma:ident [ $idx:expr ] $($field:tt)*) => {{
- $crate::dma_write!($dma, $idx, $($field)*);
+ $crate::dma_write!($dma, $idx, $($field)*)
}};
($($dma:ident).* [ $idx:expr ] $($field:tt)* ) => {{
- $crate::dma_write!($($dma).*, $idx, $($field)*);
+ $crate::dma_write!($($dma).*, $idx, $($field)*)
}};
($dma:expr, $idx: expr, = $val:expr) => {
- let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?;
- // SAFETY: `item_from_index` ensures that `item` is always a valid item.
- unsafe { $crate::dma::CoherentAllocation::field_write(&$dma, item, $val) }
+ (|| -> Result {
+ let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?;
+ // SAFETY: `item_from_index` ensures that `item` is always a valid item.
+ unsafe { $crate::dma::CoherentAllocation::field_write(&$dma, item, $val) }
+ ::core::result::Result::Ok(())
+ })()
};
($dma:expr, $idx: expr, $(.$field:ident)* = $val:expr) => {
- let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?;
- // SAFETY: `item_from_index` ensures that `item` is always a valid pointer and can be
- // dereferenced. The compiler also further validates the expression on whether `field`
- // is a member of `item` when expanded by the macro.
- unsafe {
- let ptr_field = ::core::ptr::addr_of_mut!((*item) $(.$field)*);
- $crate::dma::CoherentAllocation::field_write(&$dma, ptr_field, $val)
- }
+ (|| -> Result {
+ let item = $crate::dma::CoherentAllocation::item_from_index(&$dma, $idx)?;
+ // SAFETY: `item_from_index` ensures that `item` is always a valid pointer and can be
+ // dereferenced. The compiler also further validates the expression on whether `field`
+ // is a member of `item` when expanded by the macro.
+ unsafe {
+ let ptr_field = ::core::ptr::addr_of_mut!((*item) $(.$field)*);
+ $crate::dma::CoherentAllocation::field_write(&$dma, ptr_field, $val)
+ }
+ ::core::result::Result::Ok(())
+ })()
};
}
diff --git a/samples/rust/rust_dma.rs b/samples/rust/rust_dma.rs
index 908acd34b8db..cc09d49f2056 100644
--- a/samples/rust/rust_dma.rs
+++ b/samples/rust/rust_dma.rs
@@ -54,13 +54,9 @@ fn probe(pdev: &mut pci::Device, _info: &Self::IdInfo) -> Result<Pin<KBox<Self>>
let ca: CoherentAllocation<MyStruct> =
CoherentAllocation::alloc_coherent(pdev.as_ref(), TEST_VALUES.len(), GFP_KERNEL)?;
- || -> Result {
- for (i, value) in TEST_VALUES.into_iter().enumerate() {
- kernel::dma_write!(ca[i] = MyStruct::new(value.0, value.1));
- }
-
- Ok(())
- }()?;
+ for (i, value) in TEST_VALUES.into_iter().enumerate() {
+ kernel::dma_write!(ca[i] = MyStruct::new(value.0, value.1))?;
+ }
let drvdata = KBox::new(
Self {
@@ -78,13 +74,10 @@ impl Drop for DmaSampleDriver {
fn drop(&mut self) {
dev_info!(self.pdev.as_ref(), "Unload DMA test driver.\n");
- let _ = || -> Result {
- for (i, value) in TEST_VALUES.into_iter().enumerate() {
- assert_eq!(kernel::dma_read!(self.ca[i].h), value.0);
- assert_eq!(kernel::dma_read!(self.ca[i].b), value.1);
- }
- Ok(())
- }();
+ for (i, value) in TEST_VALUES.into_iter().enumerate() {
+ assert_eq!(kernel::dma_read!(self.ca[i].h).unwrap(), value.0);
+ assert_eq!(kernel::dma_read!(self.ca[i].b).unwrap(), value.1);
+ }
}
}
--
2.43.0
Powered by blists - more mailing lists