* [PATCH v5 1/6] rust: retain pointer mut-ness in `container_of!`
2025-03-17 14:23 [PATCH v5 0/6] rust: reduce pointer casts, enable related lints Tamir Duberstein
@ 2025-03-17 14:23 ` Tamir Duberstein
2025-03-17 14:23 ` [PATCH v5 2/6] rust: enable `clippy::ptr_as_ptr` lint Tamir Duberstein
` (6 subsequent siblings)
7 siblings, 0 replies; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-17 14:23 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
Avoid casting the input pointer to `*const _`, allowing the output
pointer to be `*mut` if the input is `*mut`. This allows a number of
`*const` to `*mut` conversions to be removed at the cost of slightly
worse ergonomics when the macro is used with a reference rather than a
pointer; the only example of this was in the macro's own doctest.
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
rust/kernel/lib.rs | 5 ++---
rust/kernel/pci.rs | 2 +-
rust/kernel/platform.rs | 2 +-
rust/kernel/rbtree.rs | 23 ++++++++++-------------
4 files changed, 14 insertions(+), 18 deletions(-)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index c92497c7c655..fc6835cc36a3 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -187,7 +187,7 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
/// }
///
/// let test = Test { a: 10, b: 20 };
-/// let b_ptr = &test.b;
+/// let b_ptr: *const _ = &test.b;
/// // SAFETY: The pointer points at the `b` field of a `Test`, so the resulting pointer will be
/// // in-bounds of the same allocation as `b_ptr`.
/// let test_alias = unsafe { container_of!(b_ptr, Test, b) };
@@ -196,9 +196,8 @@ fn panic(info: &core::panic::PanicInfo<'_>) -> ! {
#[macro_export]
macro_rules! container_of {
($ptr:expr, $type:ty, $($f:tt)*) => {{
- let ptr = $ptr as *const _ as *const u8;
let offset: usize = ::core::mem::offset_of!($type, $($f)*);
- ptr.sub(offset) as *const $type
+ $ptr.byte_sub(offset).cast::<$type>()
}}
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index f7b2743828ae..271a7690a9a0 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -364,7 +364,7 @@ pub unsafe fn from_dev(dev: ARef<device::Device>) -> Self {
fn as_raw(&self) -> *mut bindings::pci_dev {
// SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device`
// embedded in `struct pci_dev`.
- unsafe { container_of!(self.0.as_raw(), bindings::pci_dev, dev) as _ }
+ unsafe { container_of!(self.0.as_raw(), bindings::pci_dev, dev) }
}
/// Returns the PCI vendor ID.
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 1297f5292ba9..84a4ecc642a1 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -189,7 +189,7 @@ unsafe fn from_dev(dev: ARef<device::Device>) -> Self {
fn as_raw(&self) -> *mut bindings::platform_device {
// SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device`
// embedded in `struct platform_device`.
- unsafe { container_of!(self.0.as_raw(), bindings::platform_device, dev) }.cast_mut()
+ unsafe { container_of!(self.0.as_raw(), bindings::platform_device, dev) }
}
}
diff --git a/rust/kernel/rbtree.rs b/rust/kernel/rbtree.rs
index 1ea25c7092fb..27de954a0889 100644
--- a/rust/kernel/rbtree.rs
+++ b/rust/kernel/rbtree.rs
@@ -424,7 +424,7 @@ pub fn cursor_lower_bound(&mut self, key: &K) -> Option<Cursor<'_, K, V>>
while !node.is_null() {
// SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
// point to the links field of `Node<K, V>` objects.
- let this = unsafe { container_of!(node, Node<K, V>, links) }.cast_mut();
+ let this = unsafe { container_of!(node, Node<K, V>, links) };
// SAFETY: `this` is a non-null node so it is valid by the type invariants.
let this_key = unsafe { &(*this).key };
// SAFETY: `node` is a non-null node so it is valid by the type invariants.
@@ -496,7 +496,7 @@ fn drop(&mut self) {
// but it is not observable. The loop invariant is still maintained.
// SAFETY: `this` is valid per the loop invariant.
- unsafe { drop(KBox::from_raw(this.cast_mut())) };
+ unsafe { drop(KBox::from_raw(this)) };
}
}
}
@@ -761,7 +761,7 @@ pub fn remove_current(self) -> (Option<Self>, RBTreeNode<K, V>) {
let next = self.get_neighbor_raw(Direction::Next);
// SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
// point to the links field of `Node<K, V>` objects.
- let this = unsafe { container_of!(self.current.as_ptr(), Node<K, V>, links) }.cast_mut();
+ let this = unsafe { container_of!(self.current.as_ptr(), Node<K, V>, links) };
// SAFETY: `this` is valid by the type invariants as described above.
let node = unsafe { KBox::from_raw(this) };
let node = RBTreeNode { node };
@@ -806,7 +806,7 @@ fn remove_neighbor(&mut self, direction: Direction) -> Option<RBTreeNode<K, V>>
unsafe { bindings::rb_erase(neighbor, addr_of_mut!(self.tree.root)) };
// SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
// point to the links field of `Node<K, V>` objects.
- let this = unsafe { container_of!(neighbor, Node<K, V>, links) }.cast_mut();
+ let this = unsafe { container_of!(neighbor, Node<K, V>, links) };
// SAFETY: `this` is valid by the type invariants as described above.
let node = unsafe { KBox::from_raw(this) };
return Some(RBTreeNode { node });
@@ -912,7 +912,7 @@ unsafe fn to_key_value_mut<'b>(node: NonNull<bindings::rb_node>) -> (&'b K, &'b
unsafe fn to_key_value_raw<'b>(node: NonNull<bindings::rb_node>) -> (&'b K, *mut V) {
// SAFETY: By the type invariant of `Self`, all non-null `rb_node` pointers stored in `self`
// point to the links field of `Node<K, V>` objects.
- let this = unsafe { container_of!(node.as_ptr(), Node<K, V>, links) }.cast_mut();
+ let this = unsafe { container_of!(node.as_ptr(), Node<K, V>, links) };
// SAFETY: The passed `node` is the current node or a non-null neighbor,
// thus `this` is valid by the type invariants.
let k = unsafe { &(*this).key };
@@ -1021,7 +1021,7 @@ fn next(&mut self) -> Option<Self::Item> {
// SAFETY: By the type invariant of `IterRaw`, `self.next` is a valid node in an `RBTree`,
// and by the type invariant of `RBTree`, all nodes point to the links field of `Node<K, V>` objects.
- let cur = unsafe { container_of!(self.next, Node<K, V>, links) }.cast_mut();
+ let cur = unsafe { container_of!(self.next, Node<K, V>, links) };
// SAFETY: `self.next` is a valid tree node by the type invariants.
self.next = unsafe { bindings::rb_next(self.next) };
@@ -1216,7 +1216,7 @@ pub fn get_mut(&mut self) -> &mut V {
// SAFETY:
// - `self.node_links` is a valid pointer to a node in the tree.
// - We have exclusive access to the underlying tree, and can thus give out a mutable reference.
- unsafe { &mut (*(container_of!(self.node_links, Node<K, V>, links).cast_mut())).value }
+ unsafe { &mut (*(container_of!(self.node_links, Node<K, V>, links))).value }
}
/// Converts the entry into a mutable reference to its value.
@@ -1226,7 +1226,7 @@ pub fn into_mut(self) -> &'a mut V {
// SAFETY:
// - `self.node_links` is a valid pointer to a node in the tree.
// - This consumes the `&'a mut RBTree<K, V>`, therefore it can give out a mutable reference that lives for `'a`.
- unsafe { &mut (*(container_of!(self.node_links, Node<K, V>, links).cast_mut())).value }
+ unsafe { &mut (*(container_of!(self.node_links, Node<K, V>, links))).value }
}
/// Remove this entry from the [`RBTree`].
@@ -1239,9 +1239,7 @@ pub fn remove_node(self) -> RBTreeNode<K, V> {
RBTreeNode {
// SAFETY: The node was a node in the tree, but we removed it, so we can convert it
// back into a box.
- node: unsafe {
- KBox::from_raw(container_of!(self.node_links, Node<K, V>, links).cast_mut())
- },
+ node: unsafe { KBox::from_raw(container_of!(self.node_links, Node<K, V>, links)) },
}
}
@@ -1272,8 +1270,7 @@ fn replace(self, node: RBTreeNode<K, V>) -> RBTreeNode<K, V> {
// SAFETY:
// - `self.node_ptr` produces a valid pointer to a node in the tree.
// - Now that we removed this entry from the tree, we can convert the node to a box.
- let old_node =
- unsafe { KBox::from_raw(container_of!(self.node_links, Node<K, V>, links).cast_mut()) };
+ let old_node = unsafe { KBox::from_raw(container_of!(self.node_links, Node<K, V>, links)) };
RBTreeNode { node: old_node }
}
--
2.48.1
^ permalink raw reply related [flat|nested] 55+ messages in thread* [PATCH v5 2/6] rust: enable `clippy::ptr_as_ptr` lint
2025-03-17 14:23 [PATCH v5 0/6] rust: reduce pointer casts, enable related lints Tamir Duberstein
2025-03-17 14:23 ` [PATCH v5 1/6] rust: retain pointer mut-ness in `container_of!` Tamir Duberstein
@ 2025-03-17 14:23 ` Tamir Duberstein
2025-03-17 14:23 ` [PATCH v5 3/6] rust: enable `clippy::ptr_cast_constness` lint Tamir Duberstein
` (5 subsequent siblings)
7 siblings, 0 replies; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-17 14:23 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
In Rust 1.51.0, Clippy introduced the `ptr_as_ptr` lint [1]:
> Though `as` casts between raw pointers are not terrible,
> `pointer::cast` is safer because it cannot accidentally change the
> pointer's mutability, nor cast the pointer to other types like `usize`.
There are a few classes of changes required:
- Modules generated by bindgen are marked
`#[allow(clippy::ptr_as_ptr)]`.
- Inferred casts (` as _`) are replaced with `.cast()`.
- Ascribed casts (` as *... T`) are replaced with `.cast::<T>()`.
- Multistep casts from references (` as *const _ as *const T`) are
replaced with `let x: *const _ = &x;` and `.cast()` or `.cast::<T>()`
according to the previous rules. The intermediate `let` binding is
required because `(x as *const _).cast::<T>()` results in inference
failure.
- Native literal C strings are replaced with `c_str!().as_char_ptr()`.
- `*mut *mut T as _` is replaced with `let *mut *const T = (*mut *mut
T)`.cast();` since pointer to pointer can be confusing.
Apply these changes and enable the lint -- no functional change
intended.
Link: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_as_ptr [1]
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Makefile | 1 +
rust/bindings/lib.rs | 1 +
rust/kernel/alloc/allocator_test.rs | 2 +-
rust/kernel/alloc/kvec.rs | 4 ++--
rust/kernel/device.rs | 5 +++--
rust/kernel/devres.rs | 2 +-
rust/kernel/error.rs | 2 +-
rust/kernel/firmware.rs | 3 ++-
rust/kernel/fs/file.rs | 2 +-
rust/kernel/kunit.rs | 15 +++++++--------
rust/kernel/list/impl_list_item_mod.rs | 2 +-
rust/kernel/pci.rs | 2 +-
rust/kernel/platform.rs | 4 +++-
rust/kernel/print.rs | 11 +++++------
rust/kernel/seq_file.rs | 3 ++-
rust/kernel/str.rs | 2 +-
rust/kernel/sync/poll.rs | 2 +-
rust/kernel/workqueue.rs | 10 +++++-----
rust/uapi/lib.rs | 1 +
19 files changed, 40 insertions(+), 34 deletions(-)
diff --git a/Makefile b/Makefile
index 70bdbf2218fc..ec8efc8e23ba 100644
--- a/Makefile
+++ b/Makefile
@@ -483,6 +483,7 @@ export rust_common_flags := --edition=2021 \
-Wclippy::needless_continue \
-Aclippy::needless_lifetimes \
-Wclippy::no_mangle_with_rust_abi \
+ -Wclippy::ptr_as_ptr \
-Wclippy::undocumented_unsafe_blocks \
-Wclippy::unnecessary_safety_comment \
-Wclippy::unnecessary_safety_doc \
diff --git a/rust/bindings/lib.rs b/rust/bindings/lib.rs
index 014af0d1fc70..0486a32ed314 100644
--- a/rust/bindings/lib.rs
+++ b/rust/bindings/lib.rs
@@ -25,6 +25,7 @@
)]
#[allow(dead_code)]
+#[allow(clippy::ptr_as_ptr)]
#[allow(clippy::undocumented_unsafe_blocks)]
mod bindings_raw {
// Manual definition for blocklisted types.
diff --git a/rust/kernel/alloc/allocator_test.rs b/rust/kernel/alloc/allocator_test.rs
index c37d4c0c64e9..8017aa9d5213 100644
--- a/rust/kernel/alloc/allocator_test.rs
+++ b/rust/kernel/alloc/allocator_test.rs
@@ -82,7 +82,7 @@ unsafe fn realloc(
// SAFETY: Returns either NULL or a pointer to a memory allocation that satisfies or
// exceeds the given size and alignment requirements.
- let dst = unsafe { libc_aligned_alloc(layout.align(), layout.size()) } as *mut u8;
+ let dst = unsafe { libc_aligned_alloc(layout.align(), layout.size()) }.cast::<u8>();
let dst = NonNull::new(dst).ok_or(AllocError)?;
if flags.contains(__GFP_ZERO) {
diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
index ae9d072741ce..c12844764671 100644
--- a/rust/kernel/alloc/kvec.rs
+++ b/rust/kernel/alloc/kvec.rs
@@ -262,7 +262,7 @@ pub fn spare_capacity_mut(&mut self) -> &mut [MaybeUninit<T>] {
// - `self.len` is smaller than `self.capacity` and hence, the resulting pointer is
// guaranteed to be part of the same allocated object.
// - `self.len` can not overflow `isize`.
- let ptr = unsafe { self.as_mut_ptr().add(self.len) } as *mut MaybeUninit<T>;
+ let ptr = unsafe { self.as_mut_ptr().add(self.len) }.cast::<MaybeUninit<T>>();
// SAFETY: The memory between `self.len` and `self.capacity` is guaranteed to be allocated
// and valid, but uninitialized.
@@ -554,7 +554,7 @@ fn drop(&mut self) {
// - `ptr` points to memory with at least a size of `size_of::<T>() * len`,
// - all elements within `b` are initialized values of `T`,
// - `len` does not exceed `isize::MAX`.
- unsafe { Vec::from_raw_parts(ptr as _, len, len) }
+ unsafe { Vec::from_raw_parts(ptr.cast(), len, len) }
}
}
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index db2d9658ba47..9e500498835d 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -168,16 +168,17 @@ pub fn pr_dbg(&self, args: fmt::Arguments<'_>) {
/// `KERN_*`constants, for example, `KERN_CRIT`, `KERN_ALERT`, etc.
#[cfg_attr(not(CONFIG_PRINTK), allow(unused_variables))]
unsafe fn printk(&self, klevel: &[u8], msg: fmt::Arguments<'_>) {
+ let msg: *const _ = &msg;
// SAFETY: `klevel` is null-terminated and one of the kernel constants. `self.as_raw`
// is valid because `self` is valid. The "%pA" format string expects a pointer to
// `fmt::Arguments`, which is what we're passing as the last argument.
#[cfg(CONFIG_PRINTK)]
unsafe {
bindings::_dev_printk(
- klevel as *const _ as *const crate::ffi::c_char,
+ klevel.as_ptr().cast::<crate::ffi::c_char>(),
self.as_raw(),
c_str!("%pA").as_char_ptr(),
- &msg as *const _ as *const crate::ffi::c_void,
+ msg.cast::<crate::ffi::c_void>(),
)
};
}
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 942376f6f3af..3a9d998ec371 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -157,7 +157,7 @@ fn remove_action(this: &Arc<Self>) {
#[allow(clippy::missing_safety_doc)]
unsafe extern "C" fn devres_callback(ptr: *mut kernel::ffi::c_void) {
- let ptr = ptr as *mut DevresInner<T>;
+ let ptr = ptr.cast::<DevresInner<T>>();
// Devres owned this memory; now that we received the callback, drop the `Arc` and hence the
// reference.
// SAFETY: Safe, since we leaked an `Arc` reference to devm_add_action() in
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 376f6a6ae5e3..8eca5be30f2c 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -152,7 +152,7 @@ pub(crate) fn to_blk_status(self) -> bindings::blk_status_t {
/// Returns the error encoded as a pointer.
pub fn to_ptr<T>(self) -> *mut T {
// SAFETY: `self.0` is a valid error due to its invariant.
- unsafe { bindings::ERR_PTR(self.0.get() as _) as *mut _ }
+ unsafe { bindings::ERR_PTR(self.0.get() as _).cast() }
}
/// Returns a string representing the error, if one exists.
diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index c5162fdc95ff..74df815d2f4e 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -58,10 +58,11 @@ impl Firmware {
fn request_internal(name: &CStr, dev: &Device, func: FwFunc) -> Result<Self> {
let mut fw: *mut bindings::firmware = core::ptr::null_mut();
let pfw: *mut *mut bindings::firmware = &mut fw;
+ let pfw: *mut *const bindings::firmware = pfw.cast();
// SAFETY: `pfw` is a valid pointer to a NULL initialized `bindings::firmware` pointer.
// `name` and `dev` are valid as by their type invariants.
- let ret = unsafe { func.0(pfw as _, name.as_char_ptr(), dev.as_raw()) };
+ let ret = unsafe { func.0(pfw, name.as_char_ptr(), dev.as_raw()) };
if ret != 0 {
return Err(Error::from_errno(ret));
}
diff --git a/rust/kernel/fs/file.rs b/rust/kernel/fs/file.rs
index ed57e0137cdb..9e2639aee61a 100644
--- a/rust/kernel/fs/file.rs
+++ b/rust/kernel/fs/file.rs
@@ -364,7 +364,7 @@ fn deref(&self) -> &LocalFile {
//
// By the type invariants, there are no `fdget_pos` calls that did not take the
// `f_pos_lock` mutex.
- unsafe { LocalFile::from_raw_file(self as *const File as *const bindings::file) }
+ unsafe { LocalFile::from_raw_file((self as *const Self).cast()) }
}
}
diff --git a/rust/kernel/kunit.rs b/rust/kernel/kunit.rs
index 824da0e9738a..7ed2063c1af0 100644
--- a/rust/kernel/kunit.rs
+++ b/rust/kernel/kunit.rs
@@ -8,19 +8,20 @@
use core::{ffi::c_void, fmt};
+#[cfg(CONFIG_PRINTK)]
+use crate::c_str;
+
/// Prints a KUnit error-level message.
///
/// Public but hidden since it should only be used from KUnit generated code.
#[doc(hidden)]
pub fn err(args: fmt::Arguments<'_>) {
+ let args: *const _ = &args;
// SAFETY: The format string is null-terminated and the `%pA` specifier matches the argument we
// are passing.
#[cfg(CONFIG_PRINTK)]
unsafe {
- bindings::_printk(
- c"\x013%pA".as_ptr() as _,
- &args as *const _ as *const c_void,
- );
+ bindings::_printk(c_str!("\x013%pA").as_char_ptr(), args.cast::<c_void>());
}
}
@@ -29,14 +30,12 @@ pub fn err(args: fmt::Arguments<'_>) {
/// Public but hidden since it should only be used from KUnit generated code.
#[doc(hidden)]
pub fn info(args: fmt::Arguments<'_>) {
+ let args: *const _ = &args;
// SAFETY: The format string is null-terminated and the `%pA` specifier matches the argument we
// are passing.
#[cfg(CONFIG_PRINTK)]
unsafe {
- bindings::_printk(
- c"\x016%pA".as_ptr() as _,
- &args as *const _ as *const c_void,
- );
+ bindings::_printk(c_str!("\x016%pA").as_char_ptr(), args.cast::<c_void>());
}
}
diff --git a/rust/kernel/list/impl_list_item_mod.rs b/rust/kernel/list/impl_list_item_mod.rs
index a0438537cee1..1f9498c1458f 100644
--- a/rust/kernel/list/impl_list_item_mod.rs
+++ b/rust/kernel/list/impl_list_item_mod.rs
@@ -34,7 +34,7 @@ pub unsafe trait HasListLinks<const ID: u64 = 0> {
unsafe fn raw_get_list_links(ptr: *mut Self) -> *mut ListLinks<ID> {
// SAFETY: The caller promises that the pointer is valid. The implementer promises that the
// `OFFSET` constant is correct.
- unsafe { (ptr as *mut u8).add(Self::OFFSET) as *mut ListLinks<ID> }
+ unsafe { ptr.cast::<u8>().add(Self::OFFSET).cast() }
}
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 271a7690a9a0..003c9aaafb24 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -75,7 +75,7 @@ extern "C" fn probe_callback(
// Let the `struct pci_dev` own a reference of the driver's private data.
// SAFETY: By the type invariant `pdev.as_raw` returns a valid pointer to a
// `struct pci_dev`.
- unsafe { bindings::pci_set_drvdata(pdev.as_raw(), data.into_foreign() as _) };
+ unsafe { bindings::pci_set_drvdata(pdev.as_raw(), data.into_foreign().cast()) };
}
Err(err) => return Error::to_errno(err),
}
diff --git a/rust/kernel/platform.rs b/rust/kernel/platform.rs
index 84a4ecc642a1..26b2502970ef 100644
--- a/rust/kernel/platform.rs
+++ b/rust/kernel/platform.rs
@@ -66,7 +66,9 @@ extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> kernel::ff
// Let the `struct platform_device` own a reference of the driver's private data.
// SAFETY: By the type invariant `pdev.as_raw` returns a valid pointer to a
// `struct platform_device`.
- unsafe { bindings::platform_set_drvdata(pdev.as_raw(), data.into_foreign() as _) };
+ unsafe {
+ bindings::platform_set_drvdata(pdev.as_raw(), data.into_foreign().cast())
+ };
}
Err(err) => return Error::to_errno(err),
}
diff --git a/rust/kernel/print.rs b/rust/kernel/print.rs
index cf4714242e14..8ae57d2cd36c 100644
--- a/rust/kernel/print.rs
+++ b/rust/kernel/print.rs
@@ -25,7 +25,7 @@
// SAFETY: The C contract guarantees that `buf` is valid if it's less than `end`.
let mut w = unsafe { RawFormatter::from_ptrs(buf.cast(), end.cast()) };
// SAFETY: TODO.
- let _ = w.write_fmt(unsafe { *(ptr as *const fmt::Arguments<'_>) });
+ let _ = w.write_fmt(unsafe { *ptr.cast::<fmt::Arguments<'_>>() });
w.pos().cast()
}
@@ -102,6 +102,7 @@ pub unsafe fn call_printk(
module_name: &[u8],
args: fmt::Arguments<'_>,
) {
+ let args: *const _ = &args;
// `_printk` does not seem to fail in any path.
#[cfg(CONFIG_PRINTK)]
// SAFETY: TODO.
@@ -109,7 +110,7 @@ pub unsafe fn call_printk(
bindings::_printk(
format_string.as_ptr(),
module_name.as_ptr(),
- &args as *const _ as *const c_void,
+ args.cast::<c_void>(),
);
}
}
@@ -122,15 +123,13 @@ pub unsafe fn call_printk(
#[doc(hidden)]
#[cfg_attr(not(CONFIG_PRINTK), allow(unused_variables))]
pub fn call_printk_cont(args: fmt::Arguments<'_>) {
+ let args: *const _ = &args;
// `_printk` does not seem to fail in any path.
//
// SAFETY: The format string is fixed.
#[cfg(CONFIG_PRINTK)]
unsafe {
- bindings::_printk(
- format_strings::CONT.as_ptr(),
- &args as *const _ as *const c_void,
- );
+ bindings::_printk(format_strings::CONT.as_ptr(), args.cast::<c_void>());
}
}
diff --git a/rust/kernel/seq_file.rs b/rust/kernel/seq_file.rs
index 4c03881a9eba..d7a530d3eb07 100644
--- a/rust/kernel/seq_file.rs
+++ b/rust/kernel/seq_file.rs
@@ -31,12 +31,13 @@ pub unsafe fn from_raw<'a>(ptr: *mut bindings::seq_file) -> &'a SeqFile {
/// Used by the [`seq_print`] macro.
pub fn call_printf(&self, args: core::fmt::Arguments<'_>) {
+ let args: *const _ = &args;
// SAFETY: Passing a void pointer to `Arguments` is valid for `%pA`.
unsafe {
bindings::seq_printf(
self.inner.get(),
c_str!("%pA").as_char_ptr(),
- &args as *const _ as *const crate::ffi::c_void,
+ args.cast::<crate::ffi::c_void>(),
);
}
}
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 28e2201604d6..6a1a982b946d 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -191,7 +191,7 @@ pub unsafe fn from_char_ptr<'a>(ptr: *const crate::ffi::c_char) -> &'a Self {
// to a `NUL`-terminated C string.
let len = unsafe { bindings::strlen(ptr) } + 1;
// SAFETY: Lifetime guaranteed by the safety precondition.
- let bytes = unsafe { core::slice::from_raw_parts(ptr as _, len) };
+ let bytes = unsafe { core::slice::from_raw_parts(ptr.cast(), len) };
// SAFETY: As `len` is returned by `strlen`, `bytes` does not contain interior `NUL`.
// As we have added 1 to `len`, the last byte is known to be `NUL`.
unsafe { Self::from_bytes_with_nul_unchecked(bytes) }
diff --git a/rust/kernel/sync/poll.rs b/rust/kernel/sync/poll.rs
index e105477a3cb1..d04d90486b09 100644
--- a/rust/kernel/sync/poll.rs
+++ b/rust/kernel/sync/poll.rs
@@ -73,7 +73,7 @@ pub fn register_wait(&mut self, file: &File, cv: &PollCondVar) {
// be destroyed, the destructor must run. That destructor first removes all waiters,
// and then waits for an rcu grace period. Therefore, `cv.wait_queue_head` is valid for
// long enough.
- unsafe { qproc(file.as_ptr() as _, cv.wait_queue_head.get(), self.0.get()) };
+ unsafe { qproc(file.as_ptr().cast(), cv.wait_queue_head.get(), self.0.get()) };
}
}
}
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 0cd100d2aefb..8ff54105be3f 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -170,7 +170,7 @@ impl Queue {
pub unsafe fn from_raw<'a>(ptr: *const bindings::workqueue_struct) -> &'a Queue {
// SAFETY: The `Queue` type is `#[repr(transparent)]`, so the pointer cast is valid. The
// caller promises that the pointer is not dangling.
- unsafe { &*(ptr as *const Queue) }
+ unsafe { &*ptr.cast::<Queue>() }
}
/// Enqueues a work item.
@@ -457,7 +457,7 @@ fn get_work_offset(&self) -> usize {
#[inline]
unsafe fn raw_get_work(ptr: *mut Self) -> *mut Work<T, ID> {
// SAFETY: The caller promises that the pointer is valid.
- unsafe { (ptr as *mut u8).add(Self::OFFSET) as *mut Work<T, ID> }
+ unsafe { ptr.cast::<u8>().add(Self::OFFSET).cast::<Work<T, ID>>() }
}
/// Returns a pointer to the struct containing the [`Work<T, ID>`] field.
@@ -472,7 +472,7 @@ unsafe fn work_container_of(ptr: *mut Work<T, ID>) -> *mut Self
{
// SAFETY: The caller promises that the pointer points at a field of the right type in the
// right kind of struct.
- unsafe { (ptr as *mut u8).sub(Self::OFFSET) as *mut Self }
+ unsafe { ptr.cast::<u8>().sub(Self::OFFSET).cast::<Self>() }
}
}
@@ -538,7 +538,7 @@ unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Arc<T>
{
unsafe extern "C" fn run(ptr: *mut bindings::work_struct) {
// The `__enqueue` method always uses a `work_struct` stored in a `Work<T, ID>`.
- let ptr = ptr as *mut Work<T, ID>;
+ let ptr = ptr.cast::<Work<T, ID>>();
// SAFETY: This computes the pointer that `__enqueue` got from `Arc::into_raw`.
let ptr = unsafe { T::work_container_of(ptr) };
// SAFETY: This pointer comes from `Arc::into_raw` and we've been given back ownership.
@@ -591,7 +591,7 @@ unsafe impl<T, const ID: u64> WorkItemPointer<ID> for Pin<KBox<T>>
{
unsafe extern "C" fn run(ptr: *mut bindings::work_struct) {
// The `__enqueue` method always uses a `work_struct` stored in a `Work<T, ID>`.
- let ptr = ptr as *mut Work<T, ID>;
+ let ptr = ptr.cast::<Work<T, ID>>();
// SAFETY: This computes the pointer that `__enqueue` got from `Arc::into_raw`.
let ptr = unsafe { T::work_container_of(ptr) };
// SAFETY: This pointer comes from `Arc::into_raw` and we've been given back ownership.
diff --git a/rust/uapi/lib.rs b/rust/uapi/lib.rs
index 13495910271f..fe9bf7b5a306 100644
--- a/rust/uapi/lib.rs
+++ b/rust/uapi/lib.rs
@@ -15,6 +15,7 @@
#![allow(
clippy::all,
clippy::undocumented_unsafe_blocks,
+ clippy::ptr_as_ptr,
dead_code,
missing_docs,
non_camel_case_types,
--
2.48.1
^ permalink raw reply related [flat|nested] 55+ messages in thread* [PATCH v5 3/6] rust: enable `clippy::ptr_cast_constness` lint
2025-03-17 14:23 [PATCH v5 0/6] rust: reduce pointer casts, enable related lints Tamir Duberstein
2025-03-17 14:23 ` [PATCH v5 1/6] rust: retain pointer mut-ness in `container_of!` Tamir Duberstein
2025-03-17 14:23 ` [PATCH v5 2/6] rust: enable `clippy::ptr_as_ptr` lint Tamir Duberstein
@ 2025-03-17 14:23 ` Tamir Duberstein
2025-03-17 14:23 ` [PATCH v5 4/6] rust: enable `clippy::as_ptr_cast_mut` lint Tamir Duberstein
` (4 subsequent siblings)
7 siblings, 0 replies; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-17 14:23 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
In Rust 1.72.0, Clippy introduced the `ptr_cast_constness` lint [1]:
> Though `as` casts between raw pointers are not terrible,
> `pointer::cast_mut` and `pointer::cast_const` are safer because they
> cannot accidentally cast the pointer to another type.
There are only 2 affected sites:
- `*mut T as *const U as *mut U` becomes `(*mut T).cast()`
- `&self as *const Self as *mut Self` becomes a reference-to-pointer
coercion + `(*const Self).cast()`.
Apply these changes and enable the lint -- no functional change
intended.
Link: https://rust-lang.github.io/rust-clippy/master/index.html#ptr_cast_constness [1]
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Makefile | 1 +
rust/kernel/block/mq/request.rs | 5 +++--
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/Makefile b/Makefile
index ec8efc8e23ba..c62bae2b107b 100644
--- a/Makefile
+++ b/Makefile
@@ -484,6 +484,7 @@ export rust_common_flags := --edition=2021 \
-Aclippy::needless_lifetimes \
-Wclippy::no_mangle_with_rust_abi \
-Wclippy::ptr_as_ptr \
+ -Wclippy::ptr_cast_constness \
-Wclippy::undocumented_unsafe_blocks \
-Wclippy::unnecessary_safety_comment \
-Wclippy::unnecessary_safety_doc \
diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index 7943f43b9575..10c6d69be7f3 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -69,7 +69,7 @@ pub(crate) unsafe fn aref_from_raw(ptr: *mut bindings::request) -> ARef<Self> {
// INVARIANT: By the safety requirements of this function, invariants are upheld.
// SAFETY: By the safety requirement of this function, we own a
// reference count that we can pass to `ARef`.
- unsafe { ARef::from_raw(NonNull::new_unchecked(ptr as *const Self as *mut Self)) }
+ unsafe { ARef::from_raw(NonNull::new_unchecked(ptr.cast())) }
}
/// Notify the block layer that a request is going to be processed now.
@@ -151,11 +151,12 @@ pub(crate) unsafe fn wrapper_ptr(this: *mut Self) -> NonNull<RequestDataWrapper>
/// Return a reference to the [`RequestDataWrapper`] stored in the private
/// area of the request structure.
pub(crate) fn wrapper_ref(&self) -> &RequestDataWrapper {
+ let this: *const _ = self;
// SAFETY: By type invariant, `self.0` is a valid allocation. Further,
// the private data associated with this request is initialized and
// valid. The existence of `&self` guarantees that the private data is
// valid as a shared reference.
- unsafe { Self::wrapper_ptr(self as *const Self as *mut Self).as_ref() }
+ unsafe { Self::wrapper_ptr(this.cast_mut()).as_ref() }
}
}
--
2.48.1
^ permalink raw reply related [flat|nested] 55+ messages in thread* [PATCH v5 4/6] rust: enable `clippy::as_ptr_cast_mut` lint
2025-03-17 14:23 [PATCH v5 0/6] rust: reduce pointer casts, enable related lints Tamir Duberstein
` (2 preceding siblings ...)
2025-03-17 14:23 ` [PATCH v5 3/6] rust: enable `clippy::ptr_cast_constness` lint Tamir Duberstein
@ 2025-03-17 14:23 ` Tamir Duberstein
2025-03-17 14:23 ` [PATCH v5 5/6] rust: enable `clippy::as_underscore` lint Tamir Duberstein
` (3 subsequent siblings)
7 siblings, 0 replies; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-17 14:23 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
In Rust 1.66.0, Clippy introduced the `as_ptr_cast_mut` lint [1]:
> Since `as_ptr` takes a `&self`, the pointer won’t have write
> permissions unless interior mutability is used, making it unlikely
> that having it as a mutable pointer is correct.
There is only one affected callsite, and the change amounts to replacing
`as _` with `.cast_mut().cast()`. This doesn't change the semantics, but
is more descriptive of what's going on.
Apply this change and enable the lint -- no functional change intended.
Link: https://rust-lang.github.io/rust-clippy/master/index.html#as_ptr_cast_mut [1]
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Makefile | 1 +
rust/kernel/devres.rs | 2 +-
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/Makefile b/Makefile
index c62bae2b107b..bb15b86182a3 100644
--- a/Makefile
+++ b/Makefile
@@ -477,6 +477,7 @@ export rust_common_flags := --edition=2021 \
-Wrust_2018_idioms \
-Wunreachable_pub \
-Wclippy::all \
+ -Wclippy::as_ptr_cast_mut \
-Wclippy::ignored_unit_patterns \
-Wclippy::mut_mut \
-Wclippy::needless_bitwise_bool \
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 3a9d998ec371..598001157293 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -143,7 +143,7 @@ fn remove_action(this: &Arc<Self>) {
bindings::devm_remove_action_nowarn(
this.dev.as_raw(),
Some(this.callback),
- this.as_ptr() as _,
+ this.as_ptr().cast_mut().cast(),
)
};
--
2.48.1
^ permalink raw reply related [flat|nested] 55+ messages in thread* [PATCH v5 5/6] rust: enable `clippy::as_underscore` lint
2025-03-17 14:23 [PATCH v5 0/6] rust: reduce pointer casts, enable related lints Tamir Duberstein
` (3 preceding siblings ...)
2025-03-17 14:23 ` [PATCH v5 4/6] rust: enable `clippy::as_ptr_cast_mut` lint Tamir Duberstein
@ 2025-03-17 14:23 ` Tamir Duberstein
2025-03-17 14:23 ` [PATCH v5 6/6] rust: use strict provenance APIs Tamir Duberstein
` (2 subsequent siblings)
7 siblings, 0 replies; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-17 14:23 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
In Rust 1.63.0, Clippy introduced the `as_underscore` lint [1]:
> The conversion might include lossy conversion or a dangerous cast that
> might go undetected due to the type being inferred.
>
> The lint is allowed by default as using `_` is less wordy than always
> specifying the type.
Always specifying the type is especially helpful in function call
contexts where the inferred type may change at a distance. Specifying
the type also allows Clippy to spot more cases of `useless_conversion`.
The primary downside is the need to specify the type in trivial getters.
There are 4 such functions: 3 have become slightly less ergonomic, 1 was
revealed to be a `useless_conversion`.
While this doesn't eliminate unchecked `as` conversions, it makes such
conversions easier to scrutinize. It also has the slight benefit of
removing a degree of freedom on which to bikeshed. Thus apply the
changes and enable the lint -- no functional change intended.
Link: https://rust-lang.github.io/rust-clippy/master/index.html#as_underscore [1]
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
Makefile | 1 +
rust/kernel/block/mq/operations.rs | 2 +-
rust/kernel/block/mq/request.rs | 2 +-
rust/kernel/device_id.rs | 2 +-
rust/kernel/devres.rs | 15 ++++++++-------
rust/kernel/error.rs | 2 +-
rust/kernel/io.rs | 18 +++++++++---------
rust/kernel/miscdevice.rs | 2 +-
rust/kernel/of.rs | 6 +++---
rust/kernel/pci.rs | 9 ++++++---
rust/kernel/str.rs | 8 ++++----
rust/kernel/workqueue.rs | 2 +-
12 files changed, 37 insertions(+), 32 deletions(-)
diff --git a/Makefile b/Makefile
index bb15b86182a3..2af40bfed9ce 100644
--- a/Makefile
+++ b/Makefile
@@ -478,6 +478,7 @@ export rust_common_flags := --edition=2021 \
-Wunreachable_pub \
-Wclippy::all \
-Wclippy::as_ptr_cast_mut \
+ -Wclippy::as_underscore \
-Wclippy::ignored_unit_patterns \
-Wclippy::mut_mut \
-Wclippy::needless_bitwise_bool \
diff --git a/rust/kernel/block/mq/operations.rs b/rust/kernel/block/mq/operations.rs
index 864ff379dc91..d18ef55490da 100644
--- a/rust/kernel/block/mq/operations.rs
+++ b/rust/kernel/block/mq/operations.rs
@@ -101,7 +101,7 @@ impl<T: Operations> OperationsVTable<T> {
if let Err(e) = ret {
e.to_blk_status()
} else {
- bindings::BLK_STS_OK as _
+ bindings::BLK_STS_OK as u8
}
}
diff --git a/rust/kernel/block/mq/request.rs b/rust/kernel/block/mq/request.rs
index 10c6d69be7f3..bcf2b73d9189 100644
--- a/rust/kernel/block/mq/request.rs
+++ b/rust/kernel/block/mq/request.rs
@@ -125,7 +125,7 @@ pub fn end_ok(this: ARef<Self>) -> Result<(), ARef<Self>> {
// success of the call to `try_set_end` guarantees that there are no
// `ARef`s pointing to this request. Therefore it is safe to hand it
// back to the block layer.
- unsafe { bindings::blk_mq_end_request(request_ptr, bindings::BLK_STS_OK as _) };
+ unsafe { bindings::blk_mq_end_request(request_ptr, bindings::BLK_STS_OK as u8) };
Ok(())
}
diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
index e5859217a579..4063f09d76d9 100644
--- a/rust/kernel/device_id.rs
+++ b/rust/kernel/device_id.rs
@@ -82,7 +82,7 @@ impl<T: RawDeviceId, U, const N: usize> IdArray<T, U, N> {
unsafe {
raw_ids[i]
.as_mut_ptr()
- .byte_offset(T::DRIVER_DATA_OFFSET as _)
+ .byte_add(T::DRIVER_DATA_OFFSET)
.cast::<usize>()
.write(i);
}
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 598001157293..34571f992f0d 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -45,7 +45,7 @@ struct DevresInner<T> {
/// # Example
///
/// ```no_run
-/// # use kernel::{bindings, c_str, device::Device, devres::Devres, io::{Io, IoRaw}};
+/// # use kernel::{bindings, c_str, device::Device, devres::Devres, ffi::c_void, io::{Io, IoRaw}};
/// # use core::ops::Deref;
///
/// // See also [`pci::Bar`] for a real example.
@@ -59,19 +59,19 @@ struct DevresInner<T> {
/// unsafe fn new(paddr: usize) -> Result<Self>{
/// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
/// // valid for `ioremap`.
-/// let addr = unsafe { bindings::ioremap(paddr as _, SIZE as _) };
+/// let addr = unsafe { bindings::ioremap(paddr as bindings::phys_addr_t, SIZE) };
/// if addr.is_null() {
/// return Err(ENOMEM);
/// }
///
-/// Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
+/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
/// }
/// }
///
/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
/// fn drop(&mut self) {
/// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
-/// unsafe { bindings::iounmap(self.0.addr() as _); };
+/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); };
/// }
/// }
///
@@ -115,8 +115,9 @@ fn new(dev: &Device, data: T, flags: Flags) -> Result<Arc<DevresInner<T>>> {
// SAFETY: `devm_add_action` guarantees to call `Self::devres_callback` once `dev` is
// detached.
- let ret =
- unsafe { bindings::devm_add_action(dev.as_raw(), Some(inner.callback), data as _) };
+ let ret = unsafe {
+ bindings::devm_add_action(dev.as_raw(), Some(inner.callback), data.cast_mut().cast())
+ };
if ret != 0 {
// SAFETY: We just created another reference to `inner` in order to pass it to
@@ -130,7 +131,7 @@ fn new(dev: &Device, data: T, flags: Flags) -> Result<Arc<DevresInner<T>>> {
}
fn as_ptr(&self) -> *const Self {
- self as _
+ self
}
fn remove_action(this: &Arc<Self>) {
diff --git a/rust/kernel/error.rs b/rust/kernel/error.rs
index 8eca5be30f2c..340650abf446 100644
--- a/rust/kernel/error.rs
+++ b/rust/kernel/error.rs
@@ -152,7 +152,7 @@ pub(crate) fn to_blk_status(self) -> bindings::blk_status_t {
/// Returns the error encoded as a pointer.
pub fn to_ptr<T>(self) -> *mut T {
// SAFETY: `self.0` is a valid error due to its invariant.
- unsafe { bindings::ERR_PTR(self.0.get() as _).cast() }
+ unsafe { bindings::ERR_PTR(self.0.get() as isize).cast() }
}
/// Returns a string representing the error, if one exists.
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index d4a73e52e3ee..9d2aadf40edf 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -5,7 +5,7 @@
//! C header: [`include/asm-generic/io.h`](srctree/include/asm-generic/io.h)
use crate::error::{code::EINVAL, Result};
-use crate::{bindings, build_assert};
+use crate::{bindings, build_assert, ffi::c_void};
/// Raw representation of an MMIO region.
///
@@ -56,7 +56,7 @@ pub fn maxsize(&self) -> usize {
/// # Examples
///
/// ```no_run
-/// # use kernel::{bindings, io::{Io, IoRaw}};
+/// # use kernel::{bindings, ffi::c_void, io::{Io, IoRaw}};
/// # use core::ops::Deref;
///
/// // See also [`pci::Bar`] for a real example.
@@ -70,19 +70,19 @@ pub fn maxsize(&self) -> usize {
/// unsafe fn new(paddr: usize) -> Result<Self>{
/// // SAFETY: By the safety requirements of this function [`paddr`, `paddr` + `SIZE`) is
/// // valid for `ioremap`.
-/// let addr = unsafe { bindings::ioremap(paddr as _, SIZE as _) };
+/// let addr = unsafe { bindings::ioremap(paddr as bindings::phys_addr_t, SIZE) };
/// if addr.is_null() {
/// return Err(ENOMEM);
/// }
///
-/// Ok(IoMem(IoRaw::new(addr as _, SIZE)?))
+/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
/// }
/// }
///
/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
/// fn drop(&mut self) {
/// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
-/// unsafe { bindings::iounmap(self.0.addr() as _); };
+/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); };
/// }
/// }
///
@@ -119,7 +119,7 @@ pub fn $name(&self, offset: usize) -> $type_name {
let addr = self.io_addr_assert::<$type_name>(offset);
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$name(addr as _) }
+ unsafe { bindings::$name(addr as *const c_void) }
}
/// Read IO data from a given offset.
@@ -131,7 +131,7 @@ pub fn $try_name(&self, offset: usize) -> Result<$type_name> {
let addr = self.io_addr::<$type_name>(offset)?;
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- Ok(unsafe { bindings::$name(addr as _) })
+ Ok(unsafe { bindings::$name(addr as *const c_void) })
}
};
}
@@ -148,7 +148,7 @@ pub fn $name(&self, value: $type_name, offset: usize) {
let addr = self.io_addr_assert::<$type_name>(offset);
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$name(value, addr as _, ) }
+ unsafe { bindings::$name(value, addr as *mut c_void) }
}
/// Write IO data from a given offset.
@@ -160,7 +160,7 @@ pub fn $try_name(&self, value: $type_name, offset: usize) -> Result {
let addr = self.io_addr::<$type_name>(offset)?;
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$name(value, addr as _) }
+ unsafe { bindings::$name(value, addr as *mut c_void) }
Ok(())
}
};
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index e14433b2ab9d..2c66e926bffb 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -33,7 +33,7 @@ impl MiscDeviceOptions {
pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice {
// SAFETY: All zeros is valid for this C type.
let mut result: bindings::miscdevice = unsafe { MaybeUninit::zeroed().assume_init() };
- result.minor = bindings::MISC_DYNAMIC_MINOR as _;
+ result.minor = bindings::MISC_DYNAMIC_MINOR as i32;
result.name = self.name.as_char_ptr();
result.fops = create_vtable::<T>();
result
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index 04f2d8ef29cb..40d1bd13682c 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -22,7 +22,7 @@ unsafe impl RawDeviceId for DeviceId {
const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
fn index(&self) -> usize {
- self.0.data as _
+ self.0.data as usize
}
}
@@ -34,10 +34,10 @@ pub const fn new(compatible: &'static CStr) -> Self {
// SAFETY: FFI type is valid to be zero-initialized.
let mut of: bindings::of_device_id = unsafe { core::mem::zeroed() };
- // TODO: Use `clone_from_slice` once the corresponding types do match.
+ // TODO: Use `copy_from_slice` once stabilized for `const`.
let mut i = 0;
while i < src.len() {
- of.compatible[i] = src[i] as _;
+ of.compatible[i] = src[i];
i += 1;
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 003c9aaafb24..a26f154ae1b9 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -166,7 +166,7 @@ unsafe impl RawDeviceId for DeviceId {
const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::pci_device_id, driver_data);
fn index(&self) -> usize {
- self.0.driver_data as _
+ self.0.driver_data
}
}
@@ -201,7 +201,10 @@ macro_rules! pci_device_table {
/// MODULE_PCI_TABLE,
/// <MyDriver as pci::Driver>::IdInfo,
/// [
-/// (pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_REDHAT, bindings::PCI_ANY_ID as _), ())
+/// (
+/// pci::DeviceId::from_id(bindings::PCI_VENDOR_ID_REDHAT, bindings::PCI_ANY_ID as u32),
+/// (),
+/// )
/// ]
/// );
///
@@ -317,7 +320,7 @@ unsafe fn do_release(pdev: &Device, ioptr: usize, num: i32) {
// `ioptr` is valid by the safety requirements.
// `num` is valid by the safety requirements.
unsafe {
- bindings::pci_iounmap(pdev.as_raw(), ioptr as _);
+ bindings::pci_iounmap(pdev.as_raw(), ioptr as *mut kernel::ffi::c_void);
bindings::pci_release_region(pdev.as_raw(), num);
}
}
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 6a1a982b946d..0b80a119d5f0 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -692,9 +692,9 @@ fn new() -> Self {
pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
// INVARIANT: The safety requirements guarantee the type invariants.
Self {
- beg: pos as _,
- pos: pos as _,
- end: end as _,
+ beg: pos as usize,
+ pos: pos as usize,
+ end: end as usize,
}
}
@@ -719,7 +719,7 @@ pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
///
/// N.B. It may point to invalid memory.
pub(crate) fn pos(&self) -> *mut u8 {
- self.pos as _
+ self.pos as *mut u8
}
/// Returns the number of bytes written to the formatter.
diff --git a/rust/kernel/workqueue.rs b/rust/kernel/workqueue.rs
index 8ff54105be3f..d03f3440cb5a 100644
--- a/rust/kernel/workqueue.rs
+++ b/rust/kernel/workqueue.rs
@@ -198,7 +198,7 @@ pub fn enqueue<W, const ID: u64>(&self, w: W) -> W::EnqueueOutput
unsafe {
w.__enqueue(move |work_ptr| {
bindings::queue_work_on(
- bindings::wq_misc_consts_WORK_CPU_UNBOUND as _,
+ bindings::wq_misc_consts_WORK_CPU_UNBOUND as i32,
queue_ptr,
work_ptr,
)
--
2.48.1
^ permalink raw reply related [flat|nested] 55+ messages in thread* [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 14:23 [PATCH v5 0/6] rust: reduce pointer casts, enable related lints Tamir Duberstein
` (4 preceding siblings ...)
2025-03-17 14:23 ` [PATCH v5 5/6] rust: enable `clippy::as_underscore` lint Tamir Duberstein
@ 2025-03-17 14:23 ` Tamir Duberstein
2025-03-17 15:04 ` Tamir Duberstein
` (3 more replies)
2025-03-19 20:20 ` [PATCH v5 0/6] rust: reduce pointer casts, enable related lints Tamir Duberstein
2025-03-24 20:16 ` Benno Lossin
7 siblings, 4 replies; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-17 14:23 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree, Tamir Duberstein
Throughout the tree, use the strict provenance APIs stabilized in Rust
1.84.0[1]. Retain backwards-compatibility by introducing forwarding
functions at the `kernel` crate root along with polyfills for rustc <
1.84.0.
Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc <
1.84.0 as our MSRV is 1.78.0.
In the `kernel` crate, enable the strict provenance lints on rustc >=
1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing
compiler flags that are dependent on the rustc version in use.
Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1]
Suggested-by: Benno Lossin <benno.lossin@proton.me>
Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/
Signed-off-by: Tamir Duberstein <tamird@gmail.com>
---
init/Kconfig | 3 ++
rust/kernel/alloc.rs | 2 +-
rust/kernel/devres.rs | 4 +-
rust/kernel/io.rs | 14 +++----
rust/kernel/lib.rs | 108 +++++++++++++++++++++++++++++++++++++++++++++++++
rust/kernel/of.rs | 2 +-
rust/kernel/pci.rs | 4 +-
rust/kernel/str.rs | 16 +++-----
rust/kernel/uaccess.rs | 12 ++++--
9 files changed, 138 insertions(+), 27 deletions(-)
diff --git a/init/Kconfig b/init/Kconfig
index d0d021b3fa3b..82e28d6f7c3f 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -132,6 +132,9 @@ config CC_HAS_COUNTED_BY
config RUSTC_HAS_COERCE_POINTEE
def_bool RUSTC_VERSION >= 108400
+config RUSTC_HAS_STABLE_STRICT_PROVENANCE
+ def_bool RUSTC_VERSION >= 108400
+
config PAHOLE_VERSION
int
default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
diff --git a/rust/kernel/alloc.rs b/rust/kernel/alloc.rs
index fc9c9c41cd79..a1d282e48249 100644
--- a/rust/kernel/alloc.rs
+++ b/rust/kernel/alloc.rs
@@ -217,7 +217,7 @@ unsafe fn free(ptr: NonNull<u8>, layout: Layout) {
/// Returns a properly aligned dangling pointer from the given `layout`.
pub(crate) fn dangling_from_layout(layout: Layout) -> NonNull<u8> {
- let ptr = layout.align() as *mut u8;
+ let ptr = crate::without_provenance_mut(layout.align());
// SAFETY: `layout.align()` (and hence `ptr`) is guaranteed to be non-zero.
unsafe { NonNull::new_unchecked(ptr) }
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index 34571f992f0d..e8232bb771b2 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -64,14 +64,14 @@ struct DevresInner<T> {
/// return Err(ENOMEM);
/// }
///
-/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
+/// Ok(IoMem(IoRaw::new(kernel::expose_provenance(addr), SIZE)?))
/// }
/// }
///
/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
/// fn drop(&mut self) {
/// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
-/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); };
+/// unsafe { bindings::iounmap(kernel::with_exposed_provenance_mut(self.0.addr())); };
/// }
/// }
///
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 9d2aadf40edf..0a018ad7478a 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -5,7 +5,7 @@
//! C header: [`include/asm-generic/io.h`](srctree/include/asm-generic/io.h)
use crate::error::{code::EINVAL, Result};
-use crate::{bindings, build_assert, ffi::c_void};
+use crate::{bindings, build_assert};
/// Raw representation of an MMIO region.
///
@@ -75,14 +75,14 @@ pub fn maxsize(&self) -> usize {
/// return Err(ENOMEM);
/// }
///
-/// Ok(IoMem(IoRaw::new(addr as usize, SIZE)?))
+/// Ok(IoMem(IoRaw::new(kernel::expose_provenance(addr), SIZE)?))
/// }
/// }
///
/// impl<const SIZE: usize> Drop for IoMem<SIZE> {
/// fn drop(&mut self) {
/// // SAFETY: `self.0.addr()` is guaranteed to be properly mapped by `Self::new`.
-/// unsafe { bindings::iounmap(self.0.addr() as *mut c_void); };
+/// unsafe { bindings::iounmap(kernel::with_exposed_provenance_mut(self.0.addr())); };
/// }
/// }
///
@@ -119,7 +119,7 @@ pub fn $name(&self, offset: usize) -> $type_name {
let addr = self.io_addr_assert::<$type_name>(offset);
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$name(addr as *const c_void) }
+ unsafe { bindings::$name(crate::with_exposed_provenance(addr)) }
}
/// Read IO data from a given offset.
@@ -131,7 +131,7 @@ pub fn $try_name(&self, offset: usize) -> Result<$type_name> {
let addr = self.io_addr::<$type_name>(offset)?;
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- Ok(unsafe { bindings::$name(addr as *const c_void) })
+ Ok(unsafe { bindings::$name(crate::with_exposed_provenance(addr)) })
}
};
}
@@ -148,7 +148,7 @@ pub fn $name(&self, value: $type_name, offset: usize) {
let addr = self.io_addr_assert::<$type_name>(offset);
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$name(value, addr as *mut c_void) }
+ unsafe { bindings::$name(value, crate::with_exposed_provenance_mut(addr)) }
}
/// Write IO data from a given offset.
@@ -160,7 +160,7 @@ pub fn $try_name(&self, value: $type_name, offset: usize) -> Result {
let addr = self.io_addr::<$type_name>(offset)?;
// SAFETY: By the type invariant `addr` is a valid address for MMIO operations.
- unsafe { bindings::$name(value, addr as *mut c_void) }
+ unsafe { bindings::$name(value, crate::with_exposed_provenance_mut(addr)) }
Ok(())
}
};
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index fc6835cc36a3..c1b274c04a0f 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -17,6 +17,11 @@
#![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
#![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
#![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))]
+#![cfg_attr(
+ CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE,
+ feature(strict_provenance_lints),
+ deny(fuzzy_provenance_casts, lossy_provenance_casts)
+)]
#![feature(inline_const)]
#![feature(lint_reasons)]
// Stable in Rust 1.83
@@ -25,6 +30,109 @@
#![feature(const_ptr_write)]
#![feature(const_refs_to_cell)]
+#[cfg(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE)]
+#[allow(clippy::incompatible_msrv)]
+mod strict_provenance {
+ /// Gets the "address" portion of the pointer.
+ ///
+ /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
+ #[inline]
+ pub fn addr<T>(ptr: *const T) -> usize {
+ ptr.addr()
+ }
+
+ /// Exposes the "provenance" part of the pointer for future use in
+ /// [`with_exposed_provenance`] and returns the "address" portion.
+ ///
+ /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.expose_provenance.
+ #[inline]
+ pub fn expose_provenance<T>(ptr: *const T) -> usize {
+ ptr.expose_provenance()
+ }
+
+ /// Converts an address back to a pointer, picking up some previously 'exposed'
+ /// provenance.
+ ///
+ /// See https://doc.rust-lang.org/stable/core/ptr/fn.with_exposed_provenance.html.
+ #[inline]
+ pub fn with_exposed_provenance<T>(addr: usize) -> *const T {
+ core::ptr::with_exposed_provenance(addr)
+ }
+
+ /// Converts an address back to a mutable pointer, picking up some previously 'exposed'
+ /// provenance.
+ ///
+ /// See https://doc.rust-lang.org/stable/core/ptr/fn.with_exposed_provenance_mut.html
+ #[inline]
+ pub fn with_exposed_provenance_mut<T>(addr: usize) -> *mut T {
+ core::ptr::with_exposed_provenance_mut(addr)
+ }
+
+ /// Creates a pointer with the given address and no [provenance][crate::ptr#provenance].
+ ///
+ /// See https://doc.rust-lang.org/stable/core/ptr/fn.without_provenance_mut.html.
+ #[inline]
+ pub fn without_provenance_mut<T>(addr: usize) -> *mut T {
+ core::ptr::without_provenance_mut(addr)
+ }
+}
+
+#[cfg(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE))]
+mod strict_provenance {
+ /// Gets the "address" portion of the pointer.
+ ///
+ /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
+ #[inline]
+ pub fn addr<T>(ptr: *const T) -> usize {
+ // This is core's implementation from
+ // https://github.com/rust-lang/rust/commit/4291332175d12e79e6061cdc3f5dccac2e28b969 through
+ // https://github.com/rust-lang/rust/blob/1.84.0/library/core/src/ptr/const_ptr.rs#L172
+ // which is the first version that satisfies `CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE`.
+ #[allow(clippy::undocumented_unsafe_blocks)]
+ unsafe {
+ #[allow(clippy::transmutes_expressible_as_ptr_casts)]
+ core::mem::transmute(ptr.cast::<()>())
+ }
+ }
+
+ /// Exposes the "provenance" part of the pointer for future use in
+ /// [`with_exposed_provenance`] and returns the "address" portion.
+ ///
+ /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.expose_provenance.
+ #[inline]
+ pub fn expose_provenance<T>(ptr: *const T) -> usize {
+ ptr.cast::<()>() as usize
+ }
+
+ /// Converts an address back to a pointer, picking up some previously 'exposed'
+ /// provenance.
+ ///
+ /// See https://doc.rust-lang.org/stable/core/ptr/fn.with_exposed_provenance.html.
+ #[inline]
+ pub fn with_exposed_provenance<T>(addr: usize) -> *const T {
+ addr as *const T
+ }
+
+ /// Converts an address back to a mutable pointer, picking up some previously 'exposed'
+ /// provenance.
+ ///
+ /// See https://doc.rust-lang.org/stable/core/ptr/fn.with_exposed_provenance_mut.html
+ #[inline]
+ pub fn with_exposed_provenance_mut<T>(addr: usize) -> *mut T {
+ addr as *mut T
+ }
+
+ /// Creates a pointer with the given address and no [provenance][crate::ptr#provenance].
+ ///
+ /// See https://doc.rust-lang.org/stable/core/ptr/fn.without_provenance_mut.html.
+ #[inline]
+ pub fn without_provenance_mut<T>(addr: usize) -> *mut T {
+ addr as *mut T
+ }
+}
+
+pub use strict_provenance::*;
+
// Ensure conditional compilation based on the kernel configuration works;
// otherwise we may silently break things like initcall handling.
#[cfg(not(CONFIG_RUST))]
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index 40d1bd13682c..b70076d16008 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -22,7 +22,7 @@ unsafe impl RawDeviceId for DeviceId {
const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
fn index(&self) -> usize {
- self.0.data as usize
+ crate::addr(self.0.data)
}
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index a26f154ae1b9..87c9f67b3f0f 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -287,7 +287,7 @@ fn new(pdev: Device, num: u32, name: &CStr) -> Result<Self> {
// `pdev` is valid by the invariants of `Device`.
// `num` is checked for validity by a previous call to `Device::resource_len`.
// `name` is always valid.
- let ioptr: usize = unsafe { bindings::pci_iomap(pdev.as_raw(), num, 0) } as usize;
+ let ioptr = crate::expose_provenance(unsafe { bindings::pci_iomap(pdev.as_raw(), num, 0) });
if ioptr == 0 {
// SAFETY:
// `pdev` valid by the invariants of `Device`.
@@ -320,7 +320,7 @@ unsafe fn do_release(pdev: &Device, ioptr: usize, num: i32) {
// `ioptr` is valid by the safety requirements.
// `num` is valid by the safety requirements.
unsafe {
- bindings::pci_iounmap(pdev.as_raw(), ioptr as *mut kernel::ffi::c_void);
+ bindings::pci_iounmap(pdev.as_raw(), crate::with_exposed_provenance_mut(ioptr));
bindings::pci_release_region(pdev.as_raw(), num);
}
}
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 0b80a119d5f0..6bc6357293e4 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -692,9 +692,9 @@ fn new() -> Self {
pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
// INVARIANT: The safety requirements guarantee the type invariants.
Self {
- beg: pos as usize,
- pos: pos as usize,
- end: end as usize,
+ beg: crate::expose_provenance(pos),
+ pos: crate::expose_provenance(pos),
+ end: crate::expose_provenance(end),
}
}
@@ -705,7 +705,7 @@ pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
/// The memory region starting at `buf` and extending for `len` bytes must be valid for writes
/// for the lifetime of the returned [`RawFormatter`].
pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
- let pos = buf as usize;
+ let pos = crate::expose_provenance(buf);
// INVARIANT: We ensure that `end` is never less then `buf`, and the safety requirements
// guarantees that the memory region is valid for writes.
Self {
@@ -719,7 +719,7 @@ pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
///
/// N.B. It may point to invalid memory.
pub(crate) fn pos(&self) -> *mut u8 {
- self.pos as *mut u8
+ crate::with_exposed_provenance_mut(self.pos)
}
/// Returns the number of bytes written to the formatter.
@@ -741,11 +741,7 @@ fn write_str(&mut self, s: &str) -> fmt::Result {
// SAFETY: If `len_to_copy` is non-zero, then we know `pos` has not gone past `end`
// yet, so it is valid for write per the type invariants.
unsafe {
- core::ptr::copy_nonoverlapping(
- s.as_bytes().as_ptr(),
- self.pos as *mut u8,
- len_to_copy,
- )
+ core::ptr::copy_nonoverlapping(s.as_bytes().as_ptr(), self.pos(), len_to_copy)
};
}
diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
index 719b0a48ff55..96393bcf6bd7 100644
--- a/rust/kernel/uaccess.rs
+++ b/rust/kernel/uaccess.rs
@@ -226,7 +226,9 @@ pub fn read_raw(&mut self, out: &mut [MaybeUninit<u8>]) -> Result {
}
// SAFETY: `out_ptr` points into a mutable slice of length `len`, so we may write
// that many bytes to it.
- let res = unsafe { bindings::copy_from_user(out_ptr, self.ptr as *const c_void, len) };
+ let res = unsafe {
+ bindings::copy_from_user(out_ptr, crate::with_exposed_provenance(self.ptr), len)
+ };
if res != 0 {
return Err(EFAULT);
}
@@ -264,7 +266,7 @@ pub fn read<T: FromBytes>(&mut self) -> Result<T> {
let res = unsafe {
bindings::_copy_from_user(
out.as_mut_ptr().cast::<c_void>(),
- self.ptr as *const c_void,
+ crate::with_exposed_provenance(self.ptr),
len,
)
};
@@ -330,7 +332,9 @@ pub fn write_slice(&mut self, data: &[u8]) -> Result {
}
// SAFETY: `data_ptr` points into an immutable slice of length `len`, so we may read
// that many bytes from it.
- let res = unsafe { bindings::copy_to_user(self.ptr as *mut c_void, data_ptr, len) };
+ let res = unsafe {
+ bindings::copy_to_user(crate::with_exposed_provenance_mut(self.ptr), data_ptr, len)
+ };
if res != 0 {
return Err(EFAULT);
}
@@ -357,7 +361,7 @@ pub fn write<T: AsBytes>(&mut self, value: &T) -> Result {
// is a compile-time constant.
let res = unsafe {
bindings::_copy_to_user(
- self.ptr as *mut c_void,
+ crate::with_exposed_provenance_mut(self.ptr),
(value as *const T).cast::<c_void>(),
len,
)
--
2.48.1
^ permalink raw reply related [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 14:23 ` [PATCH v5 6/6] rust: use strict provenance APIs Tamir Duberstein
@ 2025-03-17 15:04 ` Tamir Duberstein
2025-03-17 17:39 ` Boqun Feng
` (2 subsequent siblings)
3 siblings, 0 replies; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-17 15:04 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Mon, Mar 17, 2025 at 10:24 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> Throughout the tree, use the strict provenance APIs stabilized in Rust
> 1.84.0[1]. Retain backwards-compatibility by introducing forwarding
> functions at the `kernel` crate root along with polyfills for rustc <
> 1.84.0.
>
> Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc <
> 1.84.0 as our MSRV is 1.78.0.
>
> In the `kernel` crate, enable the strict provenance lints on rustc >=
> 1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing
> compiler flags that are dependent on the rustc version in use.
As Benno pointed out on v4, this should probably include:
Note that the enablement of the strict provenance lints does not
extend to the `kernel` crate's doctests.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 14:23 ` [PATCH v5 6/6] rust: use strict provenance APIs Tamir Duberstein
2025-03-17 15:04 ` Tamir Duberstein
@ 2025-03-17 17:39 ` Boqun Feng
2025-03-17 18:04 ` Tamir Duberstein
2025-03-17 17:50 ` Benno Lossin
2025-03-18 12:29 ` Alice Ryhl
3 siblings, 1 reply; 55+ messages in thread
From: Boqun Feng @ 2025-03-17 17:39 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote:
[...]
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index fc6835cc36a3..c1b274c04a0f 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -17,6 +17,11 @@
> #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
> #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
> #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))]
> +#![cfg_attr(
> + CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE,
> + feature(strict_provenance_lints),
> + deny(fuzzy_provenance_casts, lossy_provenance_casts)
> +)]
> #![feature(inline_const)]
> #![feature(lint_reasons)]
> // Stable in Rust 1.83
> @@ -25,6 +30,109 @@
> #![feature(const_ptr_write)]
> #![feature(const_refs_to_cell)]
>
> +#[cfg(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE)]
> +#[allow(clippy::incompatible_msrv)]
> +mod strict_provenance {
> + /// Gets the "address" portion of the pointer.
> + ///
> + /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
> + #[inline]
> + pub fn addr<T>(ptr: *const T) -> usize {
> + ptr.addr()
> + }
> +
For addr(), I would just enable feature(strict_provenance) if
CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE=n, because that feature is
available for 1.78. Plus we may need with_addr() or map_addr() in the
future.
It saves the cost of maintaining our own *addr() and removing it when
we bump to a strict_provenance stablized version as minimal verision in
the future. Thoughts?
Regards,
Boqun
[...]
> // Ensure conditional compilation based on the kernel configuration works;
> // otherwise we may silently break things like initcall handling.
> #[cfg(not(CONFIG_RUST))]
> diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
> index 40d1bd13682c..b70076d16008 100644
> --- a/rust/kernel/of.rs
> +++ b/rust/kernel/of.rs
> @@ -22,7 +22,7 @@ unsafe impl RawDeviceId for DeviceId {
> const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
>
> fn index(&self) -> usize {
> - self.0.data as usize
> + crate::addr(self.0.data)
> }
> }
>
[...]
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 17:39 ` Boqun Feng
@ 2025-03-17 18:04 ` Tamir Duberstein
2025-03-17 18:06 ` Boqun Feng
0 siblings, 1 reply; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-17 18:04 UTC (permalink / raw)
To: Boqun Feng
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 1:39 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote:
> [...]
> > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > index fc6835cc36a3..c1b274c04a0f 100644
> > --- a/rust/kernel/lib.rs
> > +++ b/rust/kernel/lib.rs
> > @@ -17,6 +17,11 @@
> > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
> > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
> > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))]
> > +#![cfg_attr(
> > + CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE,
> > + feature(strict_provenance_lints),
> > + deny(fuzzy_provenance_casts, lossy_provenance_casts)
> > +)]
> > #![feature(inline_const)]
> > #![feature(lint_reasons)]
> > // Stable in Rust 1.83
> > @@ -25,6 +30,109 @@
> > #![feature(const_ptr_write)]
> > #![feature(const_refs_to_cell)]
> >
> > +#[cfg(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE)]
> > +#[allow(clippy::incompatible_msrv)]
> > +mod strict_provenance {
> > + /// Gets the "address" portion of the pointer.
> > + ///
> > + /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
> > + #[inline]
> > + pub fn addr<T>(ptr: *const T) -> usize {
> > + ptr.addr()
> > + }
> > +
>
> For addr(), I would just enable feature(strict_provenance) if
> CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE=n, because that feature is
> available for 1.78. Plus we may need with_addr() or map_addr() in the
> future.
We still need these stubs to avoid `clippy::incompatible_msrv`, and
we'll need those until MSRV is above 1.84.
>
> It saves the cost of maintaining our own *addr() and removing it when
> we bump to a strict_provenance stablized version as minimal verision in
> the future. Thoughts?
>
I can do this by making this particular stub unconditional. I'll do that.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 18:04 ` Tamir Duberstein
@ 2025-03-17 18:06 ` Boqun Feng
2025-03-17 18:10 ` Tamir Duberstein
0 siblings, 1 reply; 55+ messages in thread
From: Boqun Feng @ 2025-03-17 18:06 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 02:04:34PM -0400, Tamir Duberstein wrote:
> On Mon, Mar 17, 2025 at 1:39 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote:
> > [...]
> > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > > index fc6835cc36a3..c1b274c04a0f 100644
> > > --- a/rust/kernel/lib.rs
> > > +++ b/rust/kernel/lib.rs
> > > @@ -17,6 +17,11 @@
> > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
> > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
> > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))]
> > > +#![cfg_attr(
> > > + CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE,
> > > + feature(strict_provenance_lints),
> > > + deny(fuzzy_provenance_casts, lossy_provenance_casts)
> > > +)]
> > > #![feature(inline_const)]
> > > #![feature(lint_reasons)]
> > > // Stable in Rust 1.83
> > > @@ -25,6 +30,109 @@
> > > #![feature(const_ptr_write)]
> > > #![feature(const_refs_to_cell)]
> > >
> > > +#[cfg(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE)]
> > > +#[allow(clippy::incompatible_msrv)]
> > > +mod strict_provenance {
> > > + /// Gets the "address" portion of the pointer.
> > > + ///
> > > + /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
> > > + #[inline]
> > > + pub fn addr<T>(ptr: *const T) -> usize {
> > > + ptr.addr()
> > > + }
> > > +
> >
> > For addr(), I would just enable feature(strict_provenance) if
> > CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE=n, because that feature is
> > available for 1.78. Plus we may need with_addr() or map_addr() in the
> > future.
>
> We still need these stubs to avoid `clippy::incompatible_msrv`, and
> we'll need those until MSRV is above 1.84.
>
Hmm.. why? Clippy cannot work with unstable features?
Regards,
Boqun
> >
> > It saves the cost of maintaining our own *addr() and removing it when
> > we bump to a strict_provenance stablized version as minimal verision in
> > the future. Thoughts?
> >
>
> I can do this by making this particular stub unconditional. I'll do that.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 18:06 ` Boqun Feng
@ 2025-03-17 18:10 ` Tamir Duberstein
2025-03-17 18:16 ` Boqun Feng
0 siblings, 1 reply; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-17 18:10 UTC (permalink / raw)
To: Boqun Feng
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 2:06 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Mon, Mar 17, 2025 at 02:04:34PM -0400, Tamir Duberstein wrote:
> > On Mon, Mar 17, 2025 at 1:39 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > >
> > > On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote:
> > > [...]
> > > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > > > index fc6835cc36a3..c1b274c04a0f 100644
> > > > --- a/rust/kernel/lib.rs
> > > > +++ b/rust/kernel/lib.rs
> > > > @@ -17,6 +17,11 @@
> > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
> > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
> > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))]
> > > > +#![cfg_attr(
> > > > + CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE,
> > > > + feature(strict_provenance_lints),
> > > > + deny(fuzzy_provenance_casts, lossy_provenance_casts)
> > > > +)]
> > > > #![feature(inline_const)]
> > > > #![feature(lint_reasons)]
> > > > // Stable in Rust 1.83
> > > > @@ -25,6 +30,109 @@
> > > > #![feature(const_ptr_write)]
> > > > #![feature(const_refs_to_cell)]
> > > >
> > > > +#[cfg(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE)]
> > > > +#[allow(clippy::incompatible_msrv)]
> > > > +mod strict_provenance {
> > > > + /// Gets the "address" portion of the pointer.
> > > > + ///
> > > > + /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
> > > > + #[inline]
> > > > + pub fn addr<T>(ptr: *const T) -> usize {
> > > > + ptr.addr()
> > > > + }
> > > > +
> > >
> > > For addr(), I would just enable feature(strict_provenance) if
> > > CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE=n, because that feature is
> > > available for 1.78. Plus we may need with_addr() or map_addr() in the
> > > future.
> >
> > We still need these stubs to avoid `clippy::incompatible_msrv`, and
> > we'll need those until MSRV is above 1.84.
> >
>
> Hmm.. why? Clippy cannot work with unstable features?
Yes, `clippy::incompatible_msrv` doesn't pay attention to enabled
unstable features.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 18:10 ` Tamir Duberstein
@ 2025-03-17 18:16 ` Boqun Feng
2025-03-17 18:50 ` Tamir Duberstein
0 siblings, 1 reply; 55+ messages in thread
From: Boqun Feng @ 2025-03-17 18:16 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 02:10:42PM -0400, Tamir Duberstein wrote:
> On Mon, Mar 17, 2025 at 2:06 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > On Mon, Mar 17, 2025 at 02:04:34PM -0400, Tamir Duberstein wrote:
> > > On Mon, Mar 17, 2025 at 1:39 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > > >
> > > > On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote:
> > > > [...]
> > > > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > > > > index fc6835cc36a3..c1b274c04a0f 100644
> > > > > --- a/rust/kernel/lib.rs
> > > > > +++ b/rust/kernel/lib.rs
> > > > > @@ -17,6 +17,11 @@
> > > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
> > > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
> > > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))]
> > > > > +#![cfg_attr(
> > > > > + CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE,
> > > > > + feature(strict_provenance_lints),
> > > > > + deny(fuzzy_provenance_casts, lossy_provenance_casts)
> > > > > +)]
> > > > > #![feature(inline_const)]
> > > > > #![feature(lint_reasons)]
> > > > > // Stable in Rust 1.83
> > > > > @@ -25,6 +30,109 @@
> > > > > #![feature(const_ptr_write)]
> > > > > #![feature(const_refs_to_cell)]
> > > > >
> > > > > +#[cfg(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE)]
> > > > > +#[allow(clippy::incompatible_msrv)]
> > > > > +mod strict_provenance {
> > > > > + /// Gets the "address" portion of the pointer.
> > > > > + ///
> > > > > + /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
> > > > > + #[inline]
> > > > > + pub fn addr<T>(ptr: *const T) -> usize {
> > > > > + ptr.addr()
> > > > > + }
> > > > > +
> > > >
> > > > For addr(), I would just enable feature(strict_provenance) if
> > > > CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE=n, because that feature is
> > > > available for 1.78. Plus we may need with_addr() or map_addr() in the
> > > > future.
> > >
> > > We still need these stubs to avoid `clippy::incompatible_msrv`, and
> > > we'll need those until MSRV is above 1.84.
> > >
> >
> > Hmm.. why? Clippy cannot work with unstable features?
>
> Yes, `clippy::incompatible_msrv` doesn't pay attention to enabled
> unstable features.
Then we should fix clippy or how we set msrv rather adding the stub.
@Miguel?
Regards,
Boqun
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 18:16 ` Boqun Feng
@ 2025-03-17 18:50 ` Tamir Duberstein
2025-03-17 19:05 ` Tamir Duberstein
0 siblings, 1 reply; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-17 18:50 UTC (permalink / raw)
To: Boqun Feng
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Mon, Mar 17, 2025 at 02:10:42PM -0400, Tamir Duberstein wrote:
> > On Mon, Mar 17, 2025 at 2:06 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > >
> > > On Mon, Mar 17, 2025 at 02:04:34PM -0400, Tamir Duberstein wrote:
> > > > On Mon, Mar 17, 2025 at 1:39 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > > > >
> > > > > On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote:
> > > > > [...]
> > > > > > diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> > > > > > index fc6835cc36a3..c1b274c04a0f 100644
> > > > > > --- a/rust/kernel/lib.rs
> > > > > > +++ b/rust/kernel/lib.rs
> > > > > > @@ -17,6 +17,11 @@
> > > > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
> > > > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
> > > > > > #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))]
> > > > > > +#![cfg_attr(
> > > > > > + CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE,
> > > > > > + feature(strict_provenance_lints),
> > > > > > + deny(fuzzy_provenance_casts, lossy_provenance_casts)
> > > > > > +)]
> > > > > > #![feature(inline_const)]
> > > > > > #![feature(lint_reasons)]
> > > > > > // Stable in Rust 1.83
> > > > > > @@ -25,6 +30,109 @@
> > > > > > #![feature(const_ptr_write)]
> > > > > > #![feature(const_refs_to_cell)]
> > > > > >
> > > > > > +#[cfg(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE)]
> > > > > > +#[allow(clippy::incompatible_msrv)]
> > > > > > +mod strict_provenance {
> > > > > > + /// Gets the "address" portion of the pointer.
> > > > > > + ///
> > > > > > + /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
> > > > > > + #[inline]
> > > > > > + pub fn addr<T>(ptr: *const T) -> usize {
> > > > > > + ptr.addr()
> > > > > > + }
> > > > > > +
> > > > >
> > > > > For addr(), I would just enable feature(strict_provenance) if
> > > > > CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE=n, because that feature is
> > > > > available for 1.78. Plus we may need with_addr() or map_addr() in the
> > > > > future.
> > > >
> > > > We still need these stubs to avoid `clippy::incompatible_msrv`, and
> > > > we'll need those until MSRV is above 1.84.
> > > >
> > >
> > > Hmm.. why? Clippy cannot work with unstable features?
> >
> > Yes, `clippy::incompatible_msrv` doesn't pay attention to enabled
> > unstable features.
>
> Then we should fix clippy or how we set msrv rather adding the stub.
> @Miguel?
I filed https://github.com/rust-lang/rust-clippy/issues/14425.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 18:50 ` Tamir Duberstein
@ 2025-03-17 19:05 ` Tamir Duberstein
2025-03-17 20:28 ` Boqun Feng
0 siblings, 1 reply; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-17 19:05 UTC (permalink / raw)
To: Boqun Feng
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 2:50 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > Then we should fix clippy or how we set msrv rather adding the stub.
> > @Miguel?
>
> I filed https://github.com/rust-lang/rust-clippy/issues/14425.
I don't think we can wait for that to be fixed, though. Usually clippy
is distributed with rustc via rustup, so even if this is eventually
fixed, all versions between 1.84.0 and the fix will need this
workaround until MSRV is >= 1.84.0.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 19:05 ` Tamir Duberstein
@ 2025-03-17 20:28 ` Boqun Feng
2025-03-17 20:35 ` Tamir Duberstein
0 siblings, 1 reply; 55+ messages in thread
From: Boqun Feng @ 2025-03-17 20:28 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 03:05:45PM -0400, Tamir Duberstein wrote:
> On Mon, Mar 17, 2025 at 2:50 PM Tamir Duberstein <tamird@gmail.com> wrote:
> >
> > On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > >
> > > Then we should fix clippy or how we set msrv rather adding the stub.
> > > @Miguel?
> >
> > I filed https://github.com/rust-lang/rust-clippy/issues/14425.
>
> I don't think we can wait for that to be fixed, though. Usually clippy
> is distributed with rustc via rustup, so even if this is eventually
> fixed, all versions between 1.84.0 and the fix will need this
> workaround until MSRV is >= 1.84.0.
We need to take one step back to evalute this "workaround".
First, expose_provenance() and with_exposed_provenance{,_mut}() API are
clearly defined as equavilent to `as` operation [1]. Therefore, the
changes in this patch doing the conversion with expose_provenance() and
with_exposed_provenance{,_mut}() don't change anything related to
provenance in practice.
I do agree we want to use the explicit provenance API, but I don't think
we want to introduce some API that we know we will change them latter
when we bump the rustc minimal version. So the question is: are these
stubs what we want even though in the future our minimal rustc version
stablizes provenance API? If not, then the cost of this patch cannot
justify its benefits IMO.
Now let's also look into why we choose a msrv for clippy, I would guess
it's because we need to support all the versions of rustc starting at
1.78 and we want clippy to report a problem based on 1.78 even though
we're using a higher version of rustc. But for this particular case, we
use a feature that has already been stablized in a higher version of
rustc, which means the problem reported by clippy doesn't help us, nor
does it provide better code. Frankly speaking, I think we have other
ways to ensure the support of all rustc versions without a msrv for
clippy. If I was to choose, I would simply drop the msrv. But maybe I'm
missing something.
The point is tools should help us to write good and maintainable code,
we shouldn't introduce complicated structure of code just because some
tools fail to do its job.
[1]: https://doc.rust-lang.org/std/ptr/fn.with_exposed_provenance_mut.html
Regards,
Boqun
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 20:28 ` Boqun Feng
@ 2025-03-17 20:35 ` Tamir Duberstein
2025-03-17 20:46 ` Boqun Feng
0 siblings, 1 reply; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-17 20:35 UTC (permalink / raw)
To: Boqun Feng
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 4:28 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Mon, Mar 17, 2025 at 03:05:45PM -0400, Tamir Duberstein wrote:
> > On Mon, Mar 17, 2025 at 2:50 PM Tamir Duberstein <tamird@gmail.com> wrote:
> > >
> > > On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > > >
> > > > Then we should fix clippy or how we set msrv rather adding the stub.
> > > > @Miguel?
> > >
> > > I filed https://github.com/rust-lang/rust-clippy/issues/14425.
> >
> > I don't think we can wait for that to be fixed, though. Usually clippy
> > is distributed with rustc via rustup, so even if this is eventually
> > fixed, all versions between 1.84.0 and the fix will need this
> > workaround until MSRV is >= 1.84.0.
>
> We need to take one step back to evalute this "workaround".
>
> First, expose_provenance() and with_exposed_provenance{,_mut}() API are
> clearly defined as equavilent to `as` operation [1]. Therefore, the
> changes in this patch doing the conversion with expose_provenance() and
> with_exposed_provenance{,_mut}() don't change anything related to
> provenance in practice.
>
> I do agree we want to use the explicit provenance API, but I don't think
> we want to introduce some API that we know we will change them latter
> when we bump the rustc minimal version. So the question is: are these
> stubs what we want even though in the future our minimal rustc version
> stablizes provenance API? If not, then the cost of this patch cannot
> justify its benefits IMO.
>
> Now let's also look into why we choose a msrv for clippy, I would guess
> it's because we need to support all the versions of rustc starting at
> 1.78 and we want clippy to report a problem based on 1.78 even though
> we're using a higher version of rustc. But for this particular case, we
> use a feature that has already been stablized in a higher version of
> rustc, which means the problem reported by clippy doesn't help us, nor
> does it provide better code. Frankly speaking, I think we have other
> ways to ensure the support of all rustc versions without a msrv for
> clippy. If I was to choose, I would simply drop the msrv. But maybe I'm
> missing something.
>
> The point is tools should help us to write good and maintainable code,
> we shouldn't introduce complicated structure of code just because some
> tools fail to do its job.
>
> [1]: https://doc.rust-lang.org/std/ptr/fn.with_exposed_provenance_mut.html
Even if we globally disable this clippy lint, we still need stubs
because exposed_provenance was added in 1.79.0. Did your suggestion
address this? Perhaps I missed it.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 20:35 ` Tamir Duberstein
@ 2025-03-17 20:46 ` Boqun Feng
2025-03-17 20:53 ` Tamir Duberstein
0 siblings, 1 reply; 55+ messages in thread
From: Boqun Feng @ 2025-03-17 20:46 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 04:35:42PM -0400, Tamir Duberstein wrote:
> On Mon, Mar 17, 2025 at 4:28 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > On Mon, Mar 17, 2025 at 03:05:45PM -0400, Tamir Duberstein wrote:
> > > On Mon, Mar 17, 2025 at 2:50 PM Tamir Duberstein <tamird@gmail.com> wrote:
> > > >
> > > > On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > > > >
> > > > > Then we should fix clippy or how we set msrv rather adding the stub.
> > > > > @Miguel?
> > > >
> > > > I filed https://github.com/rust-lang/rust-clippy/issues/14425.
> > >
> > > I don't think we can wait for that to be fixed, though. Usually clippy
> > > is distributed with rustc via rustup, so even if this is eventually
> > > fixed, all versions between 1.84.0 and the fix will need this
> > > workaround until MSRV is >= 1.84.0.
> >
> > We need to take one step back to evalute this "workaround".
> >
> > First, expose_provenance() and with_exposed_provenance{,_mut}() API are
> > clearly defined as equavilent to `as` operation [1]. Therefore, the
> > changes in this patch doing the conversion with expose_provenance() and
> > with_exposed_provenance{,_mut}() don't change anything related to
> > provenance in practice.
> >
> > I do agree we want to use the explicit provenance API, but I don't think
> > we want to introduce some API that we know we will change them latter
> > when we bump the rustc minimal version. So the question is: are these
> > stubs what we want even though in the future our minimal rustc version
> > stablizes provenance API? If not, then the cost of this patch cannot
> > justify its benefits IMO.
> >
> > Now let's also look into why we choose a msrv for clippy, I would guess
> > it's because we need to support all the versions of rustc starting at
> > 1.78 and we want clippy to report a problem based on 1.78 even though
> > we're using a higher version of rustc. But for this particular case, we
> > use a feature that has already been stablized in a higher version of
> > rustc, which means the problem reported by clippy doesn't help us, nor
> > does it provide better code. Frankly speaking, I think we have other
> > ways to ensure the support of all rustc versions without a msrv for
> > clippy. If I was to choose, I would simply drop the msrv. But maybe I'm
> > missing something.
> >
> > The point is tools should help us to write good and maintainable code,
> > we shouldn't introduce complicated structure of code just because some
> > tools fail to do its job.
> >
> > [1]: https://doc.rust-lang.org/std/ptr/fn.with_exposed_provenance_mut.html
>
> Even if we globally disable this clippy lint, we still need stubs
> because exposed_provenance was added in 1.79.0. Did your suggestion
> address this? Perhaps I missed it.
No, I didn't.
That's a separate topic though, because I can see the argument that:
because with_exposed_provenance() is a function rather than a method, it
won't be very benefical to use ptr::with_exposed_provenance() instead of
kernel::with_exposed_provenance(), therefor these stubs of
exposed_provenance make sense to exist. But I don't think the same
argument works for ptr::{with_,map_,}addr().
Regards,
Boqun
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 20:46 ` Boqun Feng
@ 2025-03-17 20:53 ` Tamir Duberstein
2025-03-17 21:36 ` Boqun Feng
0 siblings, 1 reply; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-17 20:53 UTC (permalink / raw)
To: Boqun Feng
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 4:46 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Mon, Mar 17, 2025 at 04:35:42PM -0400, Tamir Duberstein wrote:
> > On Mon, Mar 17, 2025 at 4:28 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > >
> > > On Mon, Mar 17, 2025 at 03:05:45PM -0400, Tamir Duberstein wrote:
> > > > On Mon, Mar 17, 2025 at 2:50 PM Tamir Duberstein <tamird@gmail.com> wrote:
> > > > >
> > > > > On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > > > > >
> > > > > > Then we should fix clippy or how we set msrv rather adding the stub.
> > > > > > @Miguel?
> > > > >
> > > > > I filed https://github.com/rust-lang/rust-clippy/issues/14425.
> > > >
> > > > I don't think we can wait for that to be fixed, though. Usually clippy
> > > > is distributed with rustc via rustup, so even if this is eventually
> > > > fixed, all versions between 1.84.0 and the fix will need this
> > > > workaround until MSRV is >= 1.84.0.
> > >
> > > We need to take one step back to evalute this "workaround".
> > >
> > > First, expose_provenance() and with_exposed_provenance{,_mut}() API are
> > > clearly defined as equavilent to `as` operation [1]. Therefore, the
> > > changes in this patch doing the conversion with expose_provenance() and
> > > with_exposed_provenance{,_mut}() don't change anything related to
> > > provenance in practice.
> > >
> > > I do agree we want to use the explicit provenance API, but I don't think
> > > we want to introduce some API that we know we will change them latter
> > > when we bump the rustc minimal version. So the question is: are these
> > > stubs what we want even though in the future our minimal rustc version
> > > stablizes provenance API? If not, then the cost of this patch cannot
> > > justify its benefits IMO.
> > >
> > > Now let's also look into why we choose a msrv for clippy, I would guess
> > > it's because we need to support all the versions of rustc starting at
> > > 1.78 and we want clippy to report a problem based on 1.78 even though
> > > we're using a higher version of rustc. But for this particular case, we
> > > use a feature that has already been stablized in a higher version of
> > > rustc, which means the problem reported by clippy doesn't help us, nor
> > > does it provide better code. Frankly speaking, I think we have other
> > > ways to ensure the support of all rustc versions without a msrv for
> > > clippy. If I was to choose, I would simply drop the msrv. But maybe I'm
> > > missing something.
> > >
> > > The point is tools should help us to write good and maintainable code,
> > > we shouldn't introduce complicated structure of code just because some
> > > tools fail to do its job.
> > >
> > > [1]: https://doc.rust-lang.org/std/ptr/fn.with_exposed_provenance_mut.html
> >
> > Even if we globally disable this clippy lint, we still need stubs
> > because exposed_provenance was added in 1.79.0. Did your suggestion
> > address this? Perhaps I missed it.
>
> No, I didn't.
>
> That's a separate topic though, because I can see the argument that:
> because with_exposed_provenance() is a function rather than a method, it
> won't be very benefical to use ptr::with_exposed_provenance() instead of
> kernel::with_exposed_provenance(), therefor these stubs of
> exposed_provenance make sense to exist. But I don't think the same
> argument works for ptr::{with_,map_,}addr().
What about `pointer::expose_provenance`? It's a method that was added in 1.79.0.
We can certainly disable the clippy lint rather than add stubs for
`pointer::{with_,map_,}addr`, but it doesn't bring us to a solution
where only free functions require stubs.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 20:53 ` Tamir Duberstein
@ 2025-03-17 21:36 ` Boqun Feng
2025-03-17 23:56 ` Tamir Duberstein
2025-03-18 0:11 ` Boqun Feng
0 siblings, 2 replies; 55+ messages in thread
From: Boqun Feng @ 2025-03-17 21:36 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 04:53:18PM -0400, Tamir Duberstein wrote:
> On Mon, Mar 17, 2025 at 4:46 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> >
> > On Mon, Mar 17, 2025 at 04:35:42PM -0400, Tamir Duberstein wrote:
> > > On Mon, Mar 17, 2025 at 4:28 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > > >
> > > > On Mon, Mar 17, 2025 at 03:05:45PM -0400, Tamir Duberstein wrote:
> > > > > On Mon, Mar 17, 2025 at 2:50 PM Tamir Duberstein <tamird@gmail.com> wrote:
> > > > > >
> > > > > > On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > > > > > >
> > > > > > > Then we should fix clippy or how we set msrv rather adding the stub.
> > > > > > > @Miguel?
> > > > > >
> > > > > > I filed https://github.com/rust-lang/rust-clippy/issues/14425.
> > > > >
> > > > > I don't think we can wait for that to be fixed, though. Usually clippy
> > > > > is distributed with rustc via rustup, so even if this is eventually
> > > > > fixed, all versions between 1.84.0 and the fix will need this
> > > > > workaround until MSRV is >= 1.84.0.
> > > >
> > > > We need to take one step back to evalute this "workaround".
> > > >
> > > > First, expose_provenance() and with_exposed_provenance{,_mut}() API are
> > > > clearly defined as equavilent to `as` operation [1]. Therefore, the
> > > > changes in this patch doing the conversion with expose_provenance() and
> > > > with_exposed_provenance{,_mut}() don't change anything related to
> > > > provenance in practice.
> > > >
> > > > I do agree we want to use the explicit provenance API, but I don't think
> > > > we want to introduce some API that we know we will change them latter
> > > > when we bump the rustc minimal version. So the question is: are these
> > > > stubs what we want even though in the future our minimal rustc version
> > > > stablizes provenance API? If not, then the cost of this patch cannot
> > > > justify its benefits IMO.
> > > >
> > > > Now let's also look into why we choose a msrv for clippy, I would guess
> > > > it's because we need to support all the versions of rustc starting at
> > > > 1.78 and we want clippy to report a problem based on 1.78 even though
> > > > we're using a higher version of rustc. But for this particular case, we
> > > > use a feature that has already been stablized in a higher version of
> > > > rustc, which means the problem reported by clippy doesn't help us, nor
> > > > does it provide better code. Frankly speaking, I think we have other
> > > > ways to ensure the support of all rustc versions without a msrv for
> > > > clippy. If I was to choose, I would simply drop the msrv. But maybe I'm
> > > > missing something.
> > > >
> > > > The point is tools should help us to write good and maintainable code,
> > > > we shouldn't introduce complicated structure of code just because some
> > > > tools fail to do its job.
> > > >
> > > > [1]: https://doc.rust-lang.org/std/ptr/fn.with_exposed_provenance_mut.html
> > >
> > > Even if we globally disable this clippy lint, we still need stubs
> > > because exposed_provenance was added in 1.79.0. Did your suggestion
> > > address this? Perhaps I missed it.
> >
> > No, I didn't.
> >
> > That's a separate topic though, because I can see the argument that:
> > because with_exposed_provenance() is a function rather than a method, it
> > won't be very benefical to use ptr::with_exposed_provenance() instead of
> > kernel::with_exposed_provenance(), therefor these stubs of
> > exposed_provenance make sense to exist. But I don't think the same
> > argument works for ptr::{with_,map_,}addr().
>
> What about `pointer::expose_provenance`? It's a method that was added in 1.79.0.
>
We have a few options:
1) we can decide to use funtion-version of expose_provenance() (i.e. the
stub), if we feel the symmetry with with_exposed_provenance() is
a strong rationale. This also means we won't likely use
pointer::expose_provenance() in the future. That is, although kernel
doesn't have stable internal API, but in the foreseeable future, we
decide to use funtion-version of expose_provenance().
2) we can introduce a PtrExt trait for <1.79
pub trait PtrExt<T> {
fn expose_provenance(self) -> usize;
}
and
impl<T> PtrExt<T> for *const T {
...
}
and `PtrExt` in kernel::prelude.
(we need to #[allow(unstable_name_collisions)] to make that work)
We can also make with_exposed_provenance() use the same *Ext trick,
and remove it when we bump the minimal rustc version.
Regards,
Boqun
> We can certainly disable the clippy lint rather than add stubs for
> `pointer::{with_,map_,}addr`, but it doesn't bring us to a solution
> where only free functions require stubs.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 21:36 ` Boqun Feng
@ 2025-03-17 23:56 ` Tamir Duberstein
2025-03-18 0:14 ` Boqun Feng
2025-03-18 0:11 ` Boqun Feng
1 sibling, 1 reply; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-17 23:56 UTC (permalink / raw)
To: Boqun Feng
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 5:36 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> On Mon, Mar 17, 2025 at 04:53:18PM -0400, Tamir Duberstein wrote:
> > On Mon, Mar 17, 2025 at 4:46 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > >
> > > On Mon, Mar 17, 2025 at 04:35:42PM -0400, Tamir Duberstein wrote:
> > > > On Mon, Mar 17, 2025 at 4:28 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > > > >
> > > > > On Mon, Mar 17, 2025 at 03:05:45PM -0400, Tamir Duberstein wrote:
> > > > > > On Mon, Mar 17, 2025 at 2:50 PM Tamir Duberstein <tamird@gmail.com> wrote:
> > > > > > >
> > > > > > > On Mon, Mar 17, 2025 at 2:17 PM Boqun Feng <boqun.feng@gmail.com> wrote:
> > > > > > > >
> > > > > > > > Then we should fix clippy or how we set msrv rather adding the stub.
> > > > > > > > @Miguel?
> > > > > > >
> > > > > > > I filed https://github.com/rust-lang/rust-clippy/issues/14425.
> > > > > >
> > > > > > I don't think we can wait for that to be fixed, though. Usually clippy
> > > > > > is distributed with rustc via rustup, so even if this is eventually
> > > > > > fixed, all versions between 1.84.0 and the fix will need this
> > > > > > workaround until MSRV is >= 1.84.0.
> > > > >
> > > > > We need to take one step back to evalute this "workaround".
> > > > >
> > > > > First, expose_provenance() and with_exposed_provenance{,_mut}() API are
> > > > > clearly defined as equavilent to `as` operation [1]. Therefore, the
> > > > > changes in this patch doing the conversion with expose_provenance() and
> > > > > with_exposed_provenance{,_mut}() don't change anything related to
> > > > > provenance in practice.
> > > > >
> > > > > I do agree we want to use the explicit provenance API, but I don't think
> > > > > we want to introduce some API that we know we will change them latter
> > > > > when we bump the rustc minimal version. So the question is: are these
> > > > > stubs what we want even though in the future our minimal rustc version
> > > > > stablizes provenance API? If not, then the cost of this patch cannot
> > > > > justify its benefits IMO.
> > > > >
> > > > > Now let's also look into why we choose a msrv for clippy, I would guess
> > > > > it's because we need to support all the versions of rustc starting at
> > > > > 1.78 and we want clippy to report a problem based on 1.78 even though
> > > > > we're using a higher version of rustc. But for this particular case, we
> > > > > use a feature that has already been stablized in a higher version of
> > > > > rustc, which means the problem reported by clippy doesn't help us, nor
> > > > > does it provide better code. Frankly speaking, I think we have other
> > > > > ways to ensure the support of all rustc versions without a msrv for
> > > > > clippy. If I was to choose, I would simply drop the msrv. But maybe I'm
> > > > > missing something.
> > > > >
> > > > > The point is tools should help us to write good and maintainable code,
> > > > > we shouldn't introduce complicated structure of code just because some
> > > > > tools fail to do its job.
> > > > >
> > > > > [1]: https://doc.rust-lang.org/std/ptr/fn.with_exposed_provenance_mut.html
> > > >
> > > > Even if we globally disable this clippy lint, we still need stubs
> > > > because exposed_provenance was added in 1.79.0. Did your suggestion
> > > > address this? Perhaps I missed it.
> > >
> > > No, I didn't.
> > >
> > > That's a separate topic though, because I can see the argument that:
> > > because with_exposed_provenance() is a function rather than a method, it
> > > won't be very benefical to use ptr::with_exposed_provenance() instead of
> > > kernel::with_exposed_provenance(), therefor these stubs of
> > > exposed_provenance make sense to exist. But I don't think the same
> > > argument works for ptr::{with_,map_,}addr().
> >
> > What about `pointer::expose_provenance`? It's a method that was added in 1.79.0.
> >
>
> We have a few options:
>
> 1) we can decide to use funtion-version of expose_provenance() (i.e. the
> stub), if we feel the symmetry with with_exposed_provenance() is
> a strong rationale. This also means we won't likely use
> pointer::expose_provenance() in the future. That is, although kernel
> doesn't have stable internal API, but in the foreseeable future, we
> decide to use funtion-version of expose_provenance().
I don't think we want these stubs forever.
> 2) we can introduce a PtrExt trait for <1.79
>
> pub trait PtrExt<T> {
> fn expose_provenance(self) -> usize;
> }
>
> and
>
> impl<T> PtrExt<T> for *const T {
> ...
> }
>
> and `PtrExt` in kernel::prelude.
>
> (we need to #[allow(unstable_name_collisions)] to make that work)
I like this idea, but I can't get it to work. When both inherent and
trait methods are available, the compiler seems to prefer the inherent
method.
> We can also make with_exposed_provenance() use the same *Ext trick,
> and remove it when we bump the minimal rustc version.
This part I don't understand. What would we impl the Ext on, given
that `with_exposed_provenance` is a free function?
Option 3) take this series without the last commit, and revisit when
MSRV >= 1.79.0 or >= 1.84.0?
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 23:56 ` Tamir Duberstein
@ 2025-03-18 0:14 ` Boqun Feng
0 siblings, 0 replies; 55+ messages in thread
From: Boqun Feng @ 2025-03-18 0:14 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 07:56:09PM -0400, Tamir Duberstein wrote:
[..]
>
> Option 3) take this series without the last commit, and revisit when
> MSRV >= 1.79.0 or >= 1.84.0?
Works for me as well.
Regards,
Boqun
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 21:36 ` Boqun Feng
2025-03-17 23:56 ` Tamir Duberstein
@ 2025-03-18 0:11 ` Boqun Feng
2025-03-18 0:41 ` Tamir Duberstein
2025-03-18 9:23 ` Benno Lossin
1 sibling, 2 replies; 55+ messages in thread
From: Boqun Feng @ 2025-03-18 0:11 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 02:36:08PM -0700, Boqun Feng wrote:
[...]
> >
> > What about `pointer::expose_provenance`? It's a method that was added in 1.79.0.
> >
>
> We have a few options:
>
> 1) we can decide to use funtion-version of expose_provenance() (i.e. the
> stub), if we feel the symmetry with with_exposed_provenance() is
> a strong rationale. This also means we won't likely use
> pointer::expose_provenance() in the future. That is, although kernel
> doesn't have stable internal API, but in the foreseeable future, we
> decide to use funtion-version of expose_provenance().
>
> 2) we can introduce a PtrExt trait for <1.79
>
> pub trait PtrExt<T> {
> fn expose_provenance(self) -> usize;
> }
>
> and
>
> impl<T> PtrExt<T> for *const T {
> ...
> }
>
> and `PtrExt` in kernel::prelude.
>
> (we need to #[allow(unstable_name_collisions)] to make that work)
>
> We can also make with_exposed_provenance() use the same *Ext trick,
> and remove it when we bump the minimal rustc version.
This is probably a wrong suggestion, because with_exposed_provenance()
is a function instead of a method in Rust std.
Below is what I combined all together (based on your v5 patchset), and I
did test on 1.78, 1.79, 1.84 and 1.85 and it seems working ;-)
Regards,
Boqun
------------------------------------->8
diff --git a/init/Kconfig b/init/Kconfig
index 82e28d6f7c3f..e316b98b3612 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -135,6 +135,9 @@ config RUSTC_HAS_COERCE_POINTEE
config RUSTC_HAS_STABLE_STRICT_PROVENANCE
def_bool RUSTC_VERSION >= 108400
+config RUSTC_HAS_EXPOSED_PROVENANCE
+ def_bool RUSTC_VERSION >= 107900
+
config PAHOLE_VERSION
int
default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
index e8232bb771b2..a87a437bd9ab 100644
--- a/rust/kernel/devres.rs
+++ b/rust/kernel/devres.rs
@@ -64,7 +64,7 @@ struct DevresInner<T> {
/// return Err(ENOMEM);
/// }
///
-/// Ok(IoMem(IoRaw::new(kernel::expose_provenance(addr), SIZE)?))
+/// Ok(IoMem(IoRaw::new(addr.expose_provenance(), SIZE)?))
/// }
/// }
///
diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
index 0a018ad7478a..60c71f26d29d 100644
--- a/rust/kernel/io.rs
+++ b/rust/kernel/io.rs
@@ -75,7 +75,7 @@ pub fn maxsize(&self) -> usize {
/// return Err(ENOMEM);
/// }
///
-/// Ok(IoMem(IoRaw::new(kernel::expose_provenance(addr), SIZE)?))
+/// Ok(IoMem(IoRaw::new(addr.expose_provenance(), SIZE)?))
/// }
/// }
///
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index c1b274c04a0f..79b19e601372 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -22,6 +22,9 @@
feature(strict_provenance_lints),
deny(fuzzy_provenance_casts, lossy_provenance_casts)
)]
+#![cfg_attr(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE), feature(strict_provenance))]
+#![cfg_attr(all(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE), CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE), feature(exposed_provenance))]
+
#![feature(inline_const)]
#![feature(lint_reasons)]
// Stable in Rust 1.83
@@ -30,78 +33,24 @@
#![feature(const_ptr_write)]
#![feature(const_refs_to_cell)]
-#[cfg(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE)]
-#[allow(clippy::incompatible_msrv)]
-mod strict_provenance {
- /// Gets the "address" portion of the pointer.
- ///
- /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
- #[inline]
- pub fn addr<T>(ptr: *const T) -> usize {
- ptr.addr()
- }
-
- /// Exposes the "provenance" part of the pointer for future use in
- /// [`with_exposed_provenance`] and returns the "address" portion.
- ///
- /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.expose_provenance.
- #[inline]
- pub fn expose_provenance<T>(ptr: *const T) -> usize {
- ptr.expose_provenance()
- }
-
- /// Converts an address back to a pointer, picking up some previously 'exposed'
- /// provenance.
- ///
- /// See https://doc.rust-lang.org/stable/core/ptr/fn.with_exposed_provenance.html.
- #[inline]
- pub fn with_exposed_provenance<T>(addr: usize) -> *const T {
- core::ptr::with_exposed_provenance(addr)
- }
-
- /// Converts an address back to a mutable pointer, picking up some previously 'exposed'
- /// provenance.
- ///
- /// See https://doc.rust-lang.org/stable/core/ptr/fn.with_exposed_provenance_mut.html
- #[inline]
- pub fn with_exposed_provenance_mut<T>(addr: usize) -> *mut T {
- core::ptr::with_exposed_provenance_mut(addr)
- }
-
- /// Creates a pointer with the given address and no [provenance][crate::ptr#provenance].
- ///
- /// See https://doc.rust-lang.org/stable/core/ptr/fn.without_provenance_mut.html.
- #[inline]
- pub fn without_provenance_mut<T>(addr: usize) -> *mut T {
- core::ptr::without_provenance_mut(addr)
- }
-}
+#![allow(clippy::incompatible_msrv)]
-#[cfg(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE))]
+#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))]
mod strict_provenance {
- /// Gets the "address" portion of the pointer.
- ///
- /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
- #[inline]
- pub fn addr<T>(ptr: *const T) -> usize {
- // This is core's implementation from
- // https://github.com/rust-lang/rust/commit/4291332175d12e79e6061cdc3f5dccac2e28b969 through
- // https://github.com/rust-lang/rust/blob/1.84.0/library/core/src/ptr/const_ptr.rs#L172
- // which is the first version that satisfies `CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE`.
- #[allow(clippy::undocumented_unsafe_blocks)]
- unsafe {
- #[allow(clippy::transmutes_expressible_as_ptr_casts)]
- core::mem::transmute(ptr.cast::<()>())
- }
+ #[doc(hidden)]
+ pub trait PtrExt<T> {
+ /// Exposes the "provenance" part of the pointer for future use in
+ /// [`with_exposed_provenance`] and returns the "address" portion.
+ ///
+ /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.expose_provenance.
+ fn expose_provenance(self) -> usize;
}
- /// Exposes the "provenance" part of the pointer for future use in
- /// [`with_exposed_provenance`] and returns the "address" portion.
- ///
- /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.expose_provenance.
- #[inline]
- pub fn expose_provenance<T>(ptr: *const T) -> usize {
- ptr.cast::<()>() as usize
+ impl<T> PtrExt<T> for *const T {
+ #[inline]
+ fn expose_provenance(self) -> usize {
+ self.cast::<()>() as usize
+ }
}
/// Converts an address back to a pointer, picking up some previously 'exposed'
@@ -131,8 +80,12 @@ pub fn without_provenance_mut<T>(addr: usize) -> *mut T {
}
}
+#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))]
pub use strict_provenance::*;
+#[cfg(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE)]
+pub use core::ptr::{with_exposed_provenance, with_exposed_provenance_mut, without_provenance_mut};
+
// Ensure conditional compilation based on the kernel configuration works;
// otherwise we may silently break things like initcall handling.
#[cfg(not(CONFIG_RUST))]
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index b70076d16008..3670676071ff 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -22,7 +22,7 @@ unsafe impl RawDeviceId for DeviceId {
const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
fn index(&self) -> usize {
- crate::addr(self.0.data)
+ self.0.data.addr()
}
}
diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
index 87c9f67b3f0f..73958abdc522 100644
--- a/rust/kernel/pci.rs
+++ b/rust/kernel/pci.rs
@@ -287,7 +287,7 @@ fn new(pdev: Device, num: u32, name: &CStr) -> Result<Self> {
// `pdev` is valid by the invariants of `Device`.
// `num` is checked for validity by a previous call to `Device::resource_len`.
// `name` is always valid.
- let ioptr = crate::expose_provenance(unsafe { bindings::pci_iomap(pdev.as_raw(), num, 0) });
+ let ioptr = unsafe { bindings::pci_iomap(pdev.as_raw(), num, 0) }.expose_provenance();
if ioptr == 0 {
// SAFETY:
// `pdev` valid by the invariants of `Device`.
diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
index baa774a351ce..3ea6aa9e40e5 100644
--- a/rust/kernel/prelude.rs
+++ b/rust/kernel/prelude.rs
@@ -41,3 +41,6 @@
pub use super::init::InPlaceInit;
pub use super::current;
+
+#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))]
+pub use super::PtrExt;
diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 6bc6357293e4..d8e740267f14 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -8,6 +8,9 @@
use crate::error::{code::*, Error};
+#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))]
+use crate::PtrExt;
+
/// Byte string without UTF-8 validity guarantee.
#[repr(transparent)]
pub struct BStr([u8]);
@@ -692,9 +695,9 @@ fn new() -> Self {
pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
// INVARIANT: The safety requirements guarantee the type invariants.
Self {
- beg: crate::expose_provenance(pos),
- pos: crate::expose_provenance(pos),
- end: crate::expose_provenance(end),
+ beg: pos.expose_provenance(),
+ pos: pos.expose_provenance(),
+ end: end.expose_provenance(),
}
}
@@ -705,7 +708,7 @@ pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
/// The memory region starting at `buf` and extending for `len` bytes must be valid for writes
/// for the lifetime of the returned [`RawFormatter`].
pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
- let pos = crate::expose_provenance(buf);
+ let pos = buf.expose_provenance();
// INVARIANT: We ensure that `end` is never less then `buf`, and the safety requirements
// guarantees that the memory region is valid for writes.
Self {
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 08b6380933f5..b070da0ea972 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -226,7 +226,7 @@ $(obj)/%.lst: $(obj)/%.c FORCE
# Compile Rust sources (.rs)
# ---------------------------------------------------------------------------
-rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons
+rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons,exposed_provenance
# `--out-dir` is required to avoid temporaries being created by `rustc` in the
# current working directory, which may be not accessible in the out-of-tree
diff --git a/scripts/rustdoc_test_gen.rs b/scripts/rustdoc_test_gen.rs
index 036635fb1621..331ed32adc35 100644
--- a/scripts/rustdoc_test_gen.rs
+++ b/scripts/rustdoc_test_gen.rs
@@ -224,6 +224,8 @@ macro_rules! assert_eq {{
BufWriter::new(File::create("rust/doctests_kernel_generated.rs").unwrap()),
r#"//! `kernel` crate documentation tests.
+#![allow(clippy::incompatible_msrv)]
+
const __LOG_PREFIX: &[u8] = b"rust_doctests_kernel\0";
{rust_tests}
^ permalink raw reply related [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-18 0:11 ` Boqun Feng
@ 2025-03-18 0:41 ` Tamir Duberstein
2025-03-18 9:23 ` Benno Lossin
1 sibling, 0 replies; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-18 0:41 UTC (permalink / raw)
To: Boqun Feng
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 8:11 PM Boqun Feng <boqun.feng@gmail.com> wrote:
>
> diff --git a/scripts/rustdoc_test_gen.rs b/scripts/rustdoc_test_gen.rs
> index 036635fb1621..331ed32adc35 100644
> --- a/scripts/rustdoc_test_gen.rs
> +++ b/scripts/rustdoc_test_gen.rs
> @@ -224,6 +224,8 @@ macro_rules! assert_eq {{
> BufWriter::new(File::create("rust/doctests_kernel_generated.rs").unwrap()),
> r#"//! `kernel` crate documentation tests.
>
> +#![allow(clippy::incompatible_msrv)]
Ah, this is the reason this works for you (and the one in the kernel
root). When I said it didn't work, I was referring to not being able
to convincingly avoid these lints without disabling the check
altogether. Let's see what Miguel thinks. I agree that the options
are: extension trait + stubs/reexports + suppressing
`incompatible_msrv` or just dropping the last patch until MSRV bump.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-18 0:11 ` Boqun Feng
2025-03-18 0:41 ` Tamir Duberstein
@ 2025-03-18 9:23 ` Benno Lossin
2025-03-19 15:25 ` Boqun Feng
1 sibling, 1 reply; 55+ messages in thread
From: Benno Lossin @ 2025-03-18 9:23 UTC (permalink / raw)
To: Boqun Feng, Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Gary Guo, Björn Roy Baron, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Danilo Krummrich, Greg Kroah-Hartman,
Rafael J. Wysocki, Brendan Higgins, David Gow, Rae Moar,
Bjorn Helgaas, Luis Chamberlain, Russ Weight, Rob Herring,
Saravana Kannan, linux-kbuild, linux-kernel, rust-for-linux,
linux-kselftest, kunit-dev, linux-pci, linux-block, devicetree
On Tue Mar 18, 2025 at 1:11 AM CET, Boqun Feng wrote:
> On Mon, Mar 17, 2025 at 02:36:08PM -0700, Boqun Feng wrote:
> [...]
>> >
>> > What about `pointer::expose_provenance`? It's a method that was added in 1.79.0.
>> >
>>
>> We have a few options:
>>
>> 1) we can decide to use funtion-version of expose_provenance() (i.e. the
>> stub), if we feel the symmetry with with_exposed_provenance() is
>> a strong rationale. This also means we won't likely use
>> pointer::expose_provenance() in the future. That is, although kernel
>> doesn't have stable internal API, but in the foreseeable future, we
>> decide to use funtion-version of expose_provenance().
>>
>> 2) we can introduce a PtrExt trait for <1.79
>>
>> pub trait PtrExt<T> {
>> fn expose_provenance(self) -> usize;
>> }
>>
>> and
>>
>> impl<T> PtrExt<T> for *const T {
>> ...
>> }
>>
>> and `PtrExt` in kernel::prelude.
>>
>> (we need to #[allow(unstable_name_collisions)] to make that work)
>>
>> We can also make with_exposed_provenance() use the same *Ext trick,
>> and remove it when we bump the minimal rustc version.
>
> This is probably a wrong suggestion, because with_exposed_provenance()
> is a function instead of a method in Rust std.
>
> Below is what I combined all together (based on your v5 patchset), and I
> did test on 1.78, 1.79, 1.84 and 1.85 and it seems working ;-)
I like this a lot, I also thought that we should just disable the
`incompatible_msrv` lint. That we get rid of the stubs is a nice bonus
:)
The only annoying thing that's left IMO is that we need to conditionally
import the `PtrExt` trait, but that would need the built-in prelude
feature :(
Couple notes below.
> Regards,
> Boqun
> ------------------------------------->8
> diff --git a/init/Kconfig b/init/Kconfig
> index 82e28d6f7c3f..e316b98b3612 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -135,6 +135,9 @@ config RUSTC_HAS_COERCE_POINTEE
> config RUSTC_HAS_STABLE_STRICT_PROVENANCE
> def_bool RUSTC_VERSION >= 108400
>
> +config RUSTC_HAS_EXPOSED_PROVENANCE
> + def_bool RUSTC_VERSION >= 107900
> +
> config PAHOLE_VERSION
> int
> default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
> diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
> index e8232bb771b2..a87a437bd9ab 100644
> --- a/rust/kernel/devres.rs
> +++ b/rust/kernel/devres.rs
> @@ -64,7 +64,7 @@ struct DevresInner<T> {
> /// return Err(ENOMEM);
> /// }
> ///
> -/// Ok(IoMem(IoRaw::new(kernel::expose_provenance(addr), SIZE)?))
> +/// Ok(IoMem(IoRaw::new(addr.expose_provenance(), SIZE)?))
> /// }
> /// }
> ///
> diff --git a/rust/kernel/io.rs b/rust/kernel/io.rs
> index 0a018ad7478a..60c71f26d29d 100644
> --- a/rust/kernel/io.rs
> +++ b/rust/kernel/io.rs
> @@ -75,7 +75,7 @@ pub fn maxsize(&self) -> usize {
> /// return Err(ENOMEM);
> /// }
> ///
> -/// Ok(IoMem(IoRaw::new(kernel::expose_provenance(addr), SIZE)?))
> +/// Ok(IoMem(IoRaw::new(addr.expose_provenance(), SIZE)?))
> /// }
> /// }
> ///
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index c1b274c04a0f..79b19e601372 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -22,6 +22,9 @@
> feature(strict_provenance_lints),
> deny(fuzzy_provenance_casts, lossy_provenance_casts)
> )]
> +#![cfg_attr(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE), feature(strict_provenance))]
> +#![cfg_attr(all(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE), CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE), feature(exposed_provenance))]
> +
> #![feature(inline_const)]
> #![feature(lint_reasons)]
> // Stable in Rust 1.83
> @@ -30,78 +33,24 @@
> #![feature(const_ptr_write)]
> #![feature(const_refs_to_cell)]
>
> -#[cfg(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE)]
> -#[allow(clippy::incompatible_msrv)]
> -mod strict_provenance {
> - /// Gets the "address" portion of the pointer.
> - ///
> - /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
> - #[inline]
> - pub fn addr<T>(ptr: *const T) -> usize {
> - ptr.addr()
> - }
> -
> - /// Exposes the "provenance" part of the pointer for future use in
> - /// [`with_exposed_provenance`] and returns the "address" portion.
> - ///
> - /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.expose_provenance.
> - #[inline]
> - pub fn expose_provenance<T>(ptr: *const T) -> usize {
> - ptr.expose_provenance()
> - }
> -
> - /// Converts an address back to a pointer, picking up some previously 'exposed'
> - /// provenance.
> - ///
> - /// See https://doc.rust-lang.org/stable/core/ptr/fn.with_exposed_provenance.html.
> - #[inline]
> - pub fn with_exposed_provenance<T>(addr: usize) -> *const T {
> - core::ptr::with_exposed_provenance(addr)
> - }
> -
> - /// Converts an address back to a mutable pointer, picking up some previously 'exposed'
> - /// provenance.
> - ///
> - /// See https://doc.rust-lang.org/stable/core/ptr/fn.with_exposed_provenance_mut.html
> - #[inline]
> - pub fn with_exposed_provenance_mut<T>(addr: usize) -> *mut T {
> - core::ptr::with_exposed_provenance_mut(addr)
> - }
> -
> - /// Creates a pointer with the given address and no [provenance][crate::ptr#provenance].
> - ///
> - /// See https://doc.rust-lang.org/stable/core/ptr/fn.without_provenance_mut.html.
> - #[inline]
> - pub fn without_provenance_mut<T>(addr: usize) -> *mut T {
> - core::ptr::without_provenance_mut(addr)
> - }
> -}
> +#![allow(clippy::incompatible_msrv)]
>
> -#[cfg(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE))]
> +#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))]
> mod strict_provenance {
Since there is only a single trait and impl in here, I think we don't
need a module.
> - /// Gets the "address" portion of the pointer.
> - ///
> - /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
> - #[inline]
> - pub fn addr<T>(ptr: *const T) -> usize {
> - // This is core's implementation from
> - // https://github.com/rust-lang/rust/commit/4291332175d12e79e6061cdc3f5dccac2e28b969 through
> - // https://github.com/rust-lang/rust/blob/1.84.0/library/core/src/ptr/const_ptr.rs#L172
> - // which is the first version that satisfies `CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE`.
> - #[allow(clippy::undocumented_unsafe_blocks)]
> - unsafe {
> - #[allow(clippy::transmutes_expressible_as_ptr_casts)]
> - core::mem::transmute(ptr.cast::<()>())
> - }
> + #[doc(hidden)]
> + pub trait PtrExt<T> {
The `T` here and in the impl below probably should have a `?Sized`
bound, since that's also what the stdlib does.
> + /// Exposes the "provenance" part of the pointer for future use in
> + /// [`with_exposed_provenance`] and returns the "address" portion.
> + ///
> + /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.expose_provenance.
> + fn expose_provenance(self) -> usize;
> }
>
> - /// Exposes the "provenance" part of the pointer for future use in
> - /// [`with_exposed_provenance`] and returns the "address" portion.
> - ///
> - /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.expose_provenance.
> - #[inline]
> - pub fn expose_provenance<T>(ptr: *const T) -> usize {
> - ptr.cast::<()>() as usize
> + impl<T> PtrExt<T> for *const T {
> + #[inline]
> + fn expose_provenance(self) -> usize {
> + self.cast::<()>() as usize
> + }
> }
>
> /// Converts an address back to a pointer, picking up some previously 'exposed'
> @@ -131,8 +80,12 @@ pub fn without_provenance_mut<T>(addr: usize) -> *mut T {
> }
> }
>
> +#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))]
> pub use strict_provenance::*;
>
> +#[cfg(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE)]
> +pub use core::ptr::{with_exposed_provenance, with_exposed_provenance_mut, without_provenance_mut};
We shouldn't need this any longer, right?
---
Cheers,
Benno
> +
> // Ensure conditional compilation based on the kernel configuration works;
> // otherwise we may silently break things like initcall handling.
> #[cfg(not(CONFIG_RUST))]
> diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
> index b70076d16008..3670676071ff 100644
> --- a/rust/kernel/of.rs
> +++ b/rust/kernel/of.rs
> @@ -22,7 +22,7 @@ unsafe impl RawDeviceId for DeviceId {
> const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
>
> fn index(&self) -> usize {
> - crate::addr(self.0.data)
> + self.0.data.addr()
> }
> }
>
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 87c9f67b3f0f..73958abdc522 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -287,7 +287,7 @@ fn new(pdev: Device, num: u32, name: &CStr) -> Result<Self> {
> // `pdev` is valid by the invariants of `Device`.
> // `num` is checked for validity by a previous call to `Device::resource_len`.
> // `name` is always valid.
> - let ioptr = crate::expose_provenance(unsafe { bindings::pci_iomap(pdev.as_raw(), num, 0) });
> + let ioptr = unsafe { bindings::pci_iomap(pdev.as_raw(), num, 0) }.expose_provenance();
> if ioptr == 0 {
> // SAFETY:
> // `pdev` valid by the invariants of `Device`.
> diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
> index baa774a351ce..3ea6aa9e40e5 100644
> --- a/rust/kernel/prelude.rs
> +++ b/rust/kernel/prelude.rs
> @@ -41,3 +41,6 @@
> pub use super::init::InPlaceInit;
>
> pub use super::current;
> +
> +#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))]
> +pub use super::PtrExt;
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index 6bc6357293e4..d8e740267f14 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -8,6 +8,9 @@
>
> use crate::error::{code::*, Error};
>
> +#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))]
> +use crate::PtrExt;
> +
> /// Byte string without UTF-8 validity guarantee.
> #[repr(transparent)]
> pub struct BStr([u8]);
> @@ -692,9 +695,9 @@ fn new() -> Self {
> pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
> // INVARIANT: The safety requirements guarantee the type invariants.
> Self {
> - beg: crate::expose_provenance(pos),
> - pos: crate::expose_provenance(pos),
> - end: crate::expose_provenance(end),
> + beg: pos.expose_provenance(),
> + pos: pos.expose_provenance(),
> + end: end.expose_provenance(),
> }
> }
>
> @@ -705,7 +708,7 @@ pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
> /// The memory region starting at `buf` and extending for `len` bytes must be valid for writes
> /// for the lifetime of the returned [`RawFormatter`].
> pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
> - let pos = crate::expose_provenance(buf);
> + let pos = buf.expose_provenance();
> // INVARIANT: We ensure that `end` is never less then `buf`, and the safety requirements
> // guarantees that the memory region is valid for writes.
> Self {
> diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> index 08b6380933f5..b070da0ea972 100644
> --- a/scripts/Makefile.build
> +++ b/scripts/Makefile.build
> @@ -226,7 +226,7 @@ $(obj)/%.lst: $(obj)/%.c FORCE
> # Compile Rust sources (.rs)
> # ---------------------------------------------------------------------------
>
> -rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons
> +rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons,exposed_provenance
>
> # `--out-dir` is required to avoid temporaries being created by `rustc` in the
> # current working directory, which may be not accessible in the out-of-tree
> diff --git a/scripts/rustdoc_test_gen.rs b/scripts/rustdoc_test_gen.rs
> index 036635fb1621..331ed32adc35 100644
> --- a/scripts/rustdoc_test_gen.rs
> +++ b/scripts/rustdoc_test_gen.rs
> @@ -224,6 +224,8 @@ macro_rules! assert_eq {{
> BufWriter::new(File::create("rust/doctests_kernel_generated.rs").unwrap()),
> r#"//! `kernel` crate documentation tests.
>
> +#![allow(clippy::incompatible_msrv)]
> +
> const __LOG_PREFIX: &[u8] = b"rust_doctests_kernel\0";
>
> {rust_tests}
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-18 9:23 ` Benno Lossin
@ 2025-03-19 15:25 ` Boqun Feng
2025-03-19 20:03 ` Benno Lossin
0 siblings, 1 reply; 55+ messages in thread
From: Boqun Feng @ 2025-03-19 15:25 UTC (permalink / raw)
To: Benno Lossin
Cc: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan,
linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Tue, Mar 18, 2025 at 09:23:42AM +0000, Benno Lossin wrote:
[..]
> > +#![allow(clippy::incompatible_msrv)]
> >
> > -#[cfg(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE))]
> > +#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))]
> > mod strict_provenance {
>
> Since there is only a single trait and impl in here, I think we don't
> need a module.
>
We still need to provide stubs for with_exposed_provenance() and its
friends for rustc == 1.78, so there are a few more functions in this
module.
> > - /// Gets the "address" portion of the pointer.
> > - ///
> > - /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
> > - #[inline]
> > - pub fn addr<T>(ptr: *const T) -> usize {
> > - // This is core's implementation from
> > - // https://github.com/rust-lang/rust/commit/4291332175d12e79e6061cdc3f5dccac2e28b969 through
> > - // https://github.com/rust-lang/rust/blob/1.84.0/library/core/src/ptr/const_ptr.rs#L172
> > - // which is the first version that satisfies `CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE`.
> > - #[allow(clippy::undocumented_unsafe_blocks)]
> > - unsafe {
> > - #[allow(clippy::transmutes_expressible_as_ptr_casts)]
> > - core::mem::transmute(ptr.cast::<()>())
> > - }
> > + #[doc(hidden)]
> > + pub trait PtrExt<T> {
>
> The `T` here and in the impl below probably should have a `?Sized`
> bound, since that's also what the stdlib does.
>
Right, I was missing this.
> > + /// Exposes the "provenance" part of the pointer for future use in
> > + /// [`with_exposed_provenance`] and returns the "address" portion.
> > + ///
> > + /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.expose_provenance.
> > + fn expose_provenance(self) -> usize;
> > }
> >
> > - /// Exposes the "provenance" part of the pointer for future use in
> > - /// [`with_exposed_provenance`] and returns the "address" portion.
> > - ///
> > - /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.expose_provenance.
> > - #[inline]
> > - pub fn expose_provenance<T>(ptr: *const T) -> usize {
> > - ptr.cast::<()>() as usize
> > + impl<T> PtrExt<T> for *const T {
> > + #[inline]
> > + fn expose_provenance(self) -> usize {
> > + self.cast::<()>() as usize
> > + }
> > }
> >
> > /// Converts an address back to a pointer, picking up some previously 'exposed'
> > @@ -131,8 +80,12 @@ pub fn without_provenance_mut<T>(addr: usize) -> *mut T {
> > }
> > }
> >
> > +#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))]
> > pub use strict_provenance::*;
> >
> > +#[cfg(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE)]
> > +pub use core::ptr::{with_exposed_provenance, with_exposed_provenance_mut, without_provenance_mut};
>
> We shouldn't need this any longer, right?
>
We need re-export these for ructc >=1.79, because for rustc == 1.78 we
only have kernel::expose_provenance() and its friends, therefore
user-side can only use them.
Regards,
Boqun
> ---
> Cheers,
> Benno
>
> > +
> > // Ensure conditional compilation based on the kernel configuration works;
> > // otherwise we may silently break things like initcall handling.
> > #[cfg(not(CONFIG_RUST))]
> > diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
> > index b70076d16008..3670676071ff 100644
> > --- a/rust/kernel/of.rs
> > +++ b/rust/kernel/of.rs
> > @@ -22,7 +22,7 @@ unsafe impl RawDeviceId for DeviceId {
> > const DRIVER_DATA_OFFSET: usize = core::mem::offset_of!(bindings::of_device_id, data);
> >
> > fn index(&self) -> usize {
> > - crate::addr(self.0.data)
> > + self.0.data.addr()
> > }
> > }
> >
> > diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> > index 87c9f67b3f0f..73958abdc522 100644
> > --- a/rust/kernel/pci.rs
> > +++ b/rust/kernel/pci.rs
> > @@ -287,7 +287,7 @@ fn new(pdev: Device, num: u32, name: &CStr) -> Result<Self> {
> > // `pdev` is valid by the invariants of `Device`.
> > // `num` is checked for validity by a previous call to `Device::resource_len`.
> > // `name` is always valid.
> > - let ioptr = crate::expose_provenance(unsafe { bindings::pci_iomap(pdev.as_raw(), num, 0) });
> > + let ioptr = unsafe { bindings::pci_iomap(pdev.as_raw(), num, 0) }.expose_provenance();
> > if ioptr == 0 {
> > // SAFETY:
> > // `pdev` valid by the invariants of `Device`.
> > diff --git a/rust/kernel/prelude.rs b/rust/kernel/prelude.rs
> > index baa774a351ce..3ea6aa9e40e5 100644
> > --- a/rust/kernel/prelude.rs
> > +++ b/rust/kernel/prelude.rs
> > @@ -41,3 +41,6 @@
> > pub use super::init::InPlaceInit;
> >
> > pub use super::current;
> > +
> > +#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))]
> > +pub use super::PtrExt;
> > diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> > index 6bc6357293e4..d8e740267f14 100644
> > --- a/rust/kernel/str.rs
> > +++ b/rust/kernel/str.rs
> > @@ -8,6 +8,9 @@
> >
> > use crate::error::{code::*, Error};
> >
> > +#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))]
> > +use crate::PtrExt;
> > +
> > /// Byte string without UTF-8 validity guarantee.
> > #[repr(transparent)]
> > pub struct BStr([u8]);
> > @@ -692,9 +695,9 @@ fn new() -> Self {
> > pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
> > // INVARIANT: The safety requirements guarantee the type invariants.
> > Self {
> > - beg: crate::expose_provenance(pos),
> > - pos: crate::expose_provenance(pos),
> > - end: crate::expose_provenance(end),
> > + beg: pos.expose_provenance(),
> > + pos: pos.expose_provenance(),
> > + end: end.expose_provenance(),
> > }
> > }
> >
> > @@ -705,7 +708,7 @@ pub(crate) unsafe fn from_ptrs(pos: *mut u8, end: *mut u8) -> Self {
> > /// The memory region starting at `buf` and extending for `len` bytes must be valid for writes
> > /// for the lifetime of the returned [`RawFormatter`].
> > pub(crate) unsafe fn from_buffer(buf: *mut u8, len: usize) -> Self {
> > - let pos = crate::expose_provenance(buf);
> > + let pos = buf.expose_provenance();
> > // INVARIANT: We ensure that `end` is never less then `buf`, and the safety requirements
> > // guarantees that the memory region is valid for writes.
> > Self {
> > diff --git a/scripts/Makefile.build b/scripts/Makefile.build
> > index 08b6380933f5..b070da0ea972 100644
> > --- a/scripts/Makefile.build
> > +++ b/scripts/Makefile.build
> > @@ -226,7 +226,7 @@ $(obj)/%.lst: $(obj)/%.c FORCE
> > # Compile Rust sources (.rs)
> > # ---------------------------------------------------------------------------
> >
> > -rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons
> > +rust_allowed_features := asm_const,asm_goto,arbitrary_self_types,lint_reasons,exposed_provenance
> >
> > # `--out-dir` is required to avoid temporaries being created by `rustc` in the
> > # current working directory, which may be not accessible in the out-of-tree
> > diff --git a/scripts/rustdoc_test_gen.rs b/scripts/rustdoc_test_gen.rs
> > index 036635fb1621..331ed32adc35 100644
> > --- a/scripts/rustdoc_test_gen.rs
> > +++ b/scripts/rustdoc_test_gen.rs
> > @@ -224,6 +224,8 @@ macro_rules! assert_eq {{
> > BufWriter::new(File::create("rust/doctests_kernel_generated.rs").unwrap()),
> > r#"//! `kernel` crate documentation tests.
> >
> > +#![allow(clippy::incompatible_msrv)]
> > +
> > const __LOG_PREFIX: &[u8] = b"rust_doctests_kernel\0";
> >
> > {rust_tests}
>
>
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-19 15:25 ` Boqun Feng
@ 2025-03-19 20:03 ` Benno Lossin
0 siblings, 0 replies; 55+ messages in thread
From: Benno Lossin @ 2025-03-19 20:03 UTC (permalink / raw)
To: Boqun Feng
Cc: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan,
linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Wed Mar 19, 2025 at 4:25 PM CET, Boqun Feng wrote:
> On Tue, Mar 18, 2025 at 09:23:42AM +0000, Benno Lossin wrote:
> [..]
>> > +#![allow(clippy::incompatible_msrv)]
>> >
>> > -#[cfg(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE))]
>> > +#[cfg(not(CONFIG_RUSTC_HAS_EXPOSED_PROVENANCE))]
>> > mod strict_provenance {
>>
>> Since there is only a single trait and impl in here, I think we don't
>> need a module.
>
> We still need to provide stubs for with_exposed_provenance() and its
> friends for rustc == 1.78, so there are a few more functions in this
> module.
Ah I see, that's okay then.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 14:23 ` [PATCH v5 6/6] rust: use strict provenance APIs Tamir Duberstein
2025-03-17 15:04 ` Tamir Duberstein
2025-03-17 17:39 ` Boqun Feng
@ 2025-03-17 17:50 ` Benno Lossin
2025-03-17 18:31 ` Tamir Duberstein
2025-03-18 12:29 ` Alice Ryhl
3 siblings, 1 reply; 55+ messages in thread
From: Benno Lossin @ 2025-03-17 17:50 UTC (permalink / raw)
To: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Mon Mar 17, 2025 at 3:23 PM CET, Tamir Duberstein wrote:
> Throughout the tree, use the strict provenance APIs stabilized in Rust
> 1.84.0[1]. Retain backwards-compatibility by introducing forwarding
> functions at the `kernel` crate root along with polyfills for rustc <
> 1.84.0.
>
> Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc <
> 1.84.0 as our MSRV is 1.78.0.
>
> In the `kernel` crate, enable the strict provenance lints on rustc >=
> 1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing
> compiler flags that are dependent on the rustc version in use.
>
> Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1]
> Suggested-by: Benno Lossin <benno.lossin@proton.me>
> Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
One comment below, with that fixed:
Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> ---
> init/Kconfig | 3 ++
> rust/kernel/alloc.rs | 2 +-
> rust/kernel/devres.rs | 4 +-
> rust/kernel/io.rs | 14 +++----
> rust/kernel/lib.rs | 108 +++++++++++++++++++++++++++++++++++++++++++++++++
> rust/kernel/of.rs | 2 +-
> rust/kernel/pci.rs | 4 +-
> rust/kernel/str.rs | 16 +++-----
> rust/kernel/uaccess.rs | 12 ++++--
> 9 files changed, 138 insertions(+), 27 deletions(-)
> +#[cfg(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE))]
> +mod strict_provenance {
> + /// Gets the "address" portion of the pointer.
> + ///
> + /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
> + #[inline]
> + pub fn addr<T>(ptr: *const T) -> usize {
> + // This is core's implementation from
> + // https://github.com/rust-lang/rust/commit/4291332175d12e79e6061cdc3f5dccac2e28b969 through
> + // https://github.com/rust-lang/rust/blob/1.84.0/library/core/src/ptr/const_ptr.rs#L172
> + // which is the first version that satisfies `CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE`.
> + #[allow(clippy::undocumented_unsafe_blocks)]
> + unsafe {
> + #[allow(clippy::transmutes_expressible_as_ptr_casts)]
> + core::mem::transmute(ptr.cast::<()>())
> + }
I think we should just use `ptr as usize` here instead. It's going away
at some point and it will only affect optimizations (I don't even know
if they exist at the moment) of old versions.
---
Cheers,
Benno
> + }
> +
> + /// Exposes the "provenance" part of the pointer for future use in
> + /// [`with_exposed_provenance`] and returns the "address" portion.
> + ///
> + /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.expose_provenance.
> + #[inline]
> + pub fn expose_provenance<T>(ptr: *const T) -> usize {
> + ptr.cast::<()>() as usize
> + }
> +
> + /// Converts an address back to a pointer, picking up some previously 'exposed'
> + /// provenance.
> + ///
> + /// See https://doc.rust-lang.org/stable/core/ptr/fn.with_exposed_provenance.html.
> + #[inline]
> + pub fn with_exposed_provenance<T>(addr: usize) -> *const T {
> + addr as *const T
> + }
> +
> + /// Converts an address back to a mutable pointer, picking up some previously 'exposed'
> + /// provenance.
> + ///
> + /// See https://doc.rust-lang.org/stable/core/ptr/fn.with_exposed_provenance_mut.html
> + #[inline]
> + pub fn with_exposed_provenance_mut<T>(addr: usize) -> *mut T {
> + addr as *mut T
> + }
> +
> + /// Creates a pointer with the given address and no [provenance][crate::ptr#provenance].
> + ///
> + /// See https://doc.rust-lang.org/stable/core/ptr/fn.without_provenance_mut.html.
> + #[inline]
> + pub fn without_provenance_mut<T>(addr: usize) -> *mut T {
> + addr as *mut T
> + }
> +}
> +
> +pub use strict_provenance::*;
> +
> // Ensure conditional compilation based on the kernel configuration works;
> // otherwise we may silently break things like initcall handling.
> #[cfg(not(CONFIG_RUST))]
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 17:50 ` Benno Lossin
@ 2025-03-17 18:31 ` Tamir Duberstein
2025-03-17 18:33 ` Tamir Duberstein
0 siblings, 1 reply; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-17 18:31 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 1:50 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Mon Mar 17, 2025 at 3:23 PM CET, Tamir Duberstein wrote:
> > Throughout the tree, use the strict provenance APIs stabilized in Rust
> > 1.84.0[1]. Retain backwards-compatibility by introducing forwarding
> > functions at the `kernel` crate root along with polyfills for rustc <
> > 1.84.0.
> >
> > Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc <
> > 1.84.0 as our MSRV is 1.78.0.
> >
> > In the `kernel` crate, enable the strict provenance lints on rustc >=
> > 1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing
> > compiler flags that are dependent on the rustc version in use.
> >
> > Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1]
> > Suggested-by: Benno Lossin <benno.lossin@proton.me>
> > Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
>
> One comment below, with that fixed:
>
> Reviewed-by: Benno Lossin <benno.lossin@proton.me>
>
> > ---
> > init/Kconfig | 3 ++
> > rust/kernel/alloc.rs | 2 +-
> > rust/kernel/devres.rs | 4 +-
> > rust/kernel/io.rs | 14 +++----
> > rust/kernel/lib.rs | 108 +++++++++++++++++++++++++++++++++++++++++++++++++
> > rust/kernel/of.rs | 2 +-
> > rust/kernel/pci.rs | 4 +-
> > rust/kernel/str.rs | 16 +++-----
> > rust/kernel/uaccess.rs | 12 ++++--
> > 9 files changed, 138 insertions(+), 27 deletions(-)
>
>
> > +#[cfg(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE))]
> > +mod strict_provenance {
> > + /// Gets the "address" portion of the pointer.
> > + ///
> > + /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
> > + #[inline]
> > + pub fn addr<T>(ptr: *const T) -> usize {
> > + // This is core's implementation from
> > + // https://github.com/rust-lang/rust/commit/4291332175d12e79e6061cdc3f5dccac2e28b969 through
> > + // https://github.com/rust-lang/rust/blob/1.84.0/library/core/src/ptr/const_ptr.rs#L172
> > + // which is the first version that satisfies `CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE`.
> > + #[allow(clippy::undocumented_unsafe_blocks)]
> > + unsafe {
> > + #[allow(clippy::transmutes_expressible_as_ptr_casts)]
> > + core::mem::transmute(ptr.cast::<()>())
> > + }
>
> I think we should just use `ptr as usize` here instead. It's going away
> at some point and it will only affect optimizations (I don't even know
> if they exist at the moment) of old versions.
Why get cute? I'd rather defer to the standard library.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 18:31 ` Tamir Duberstein
@ 2025-03-17 18:33 ` Tamir Duberstein
0 siblings, 0 replies; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-17 18:33 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 2:31 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Mon, Mar 17, 2025 at 1:50 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >
> > On Mon Mar 17, 2025 at 3:23 PM CET, Tamir Duberstein wrote:
> > > Throughout the tree, use the strict provenance APIs stabilized in Rust
> > > 1.84.0[1]. Retain backwards-compatibility by introducing forwarding
> > > functions at the `kernel` crate root along with polyfills for rustc <
> > > 1.84.0.
> > >
> > > Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc <
> > > 1.84.0 as our MSRV is 1.78.0.
> > >
> > > In the `kernel` crate, enable the strict provenance lints on rustc >=
> > > 1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing
> > > compiler flags that are dependent on the rustc version in use.
> > >
> > > Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1]
> > > Suggested-by: Benno Lossin <benno.lossin@proton.me>
> > > Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/
> > > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> >
> > One comment below, with that fixed:
> >
> > Reviewed-by: Benno Lossin <benno.lossin@proton.me>
> >
> > > ---
> > > init/Kconfig | 3 ++
> > > rust/kernel/alloc.rs | 2 +-
> > > rust/kernel/devres.rs | 4 +-
> > > rust/kernel/io.rs | 14 +++----
> > > rust/kernel/lib.rs | 108 +++++++++++++++++++++++++++++++++++++++++++++++++
> > > rust/kernel/of.rs | 2 +-
> > > rust/kernel/pci.rs | 4 +-
> > > rust/kernel/str.rs | 16 +++-----
> > > rust/kernel/uaccess.rs | 12 ++++--
> > > 9 files changed, 138 insertions(+), 27 deletions(-)
> >
> >
> > > +#[cfg(not(CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE))]
> > > +mod strict_provenance {
> > > + /// Gets the "address" portion of the pointer.
> > > + ///
> > > + /// See https://doc.rust-lang.org/stable/core/primitive.pointer.html#method.addr.
> > > + #[inline]
> > > + pub fn addr<T>(ptr: *const T) -> usize {
> > > + // This is core's implementation from
> > > + // https://github.com/rust-lang/rust/commit/4291332175d12e79e6061cdc3f5dccac2e28b969 through
> > > + // https://github.com/rust-lang/rust/blob/1.84.0/library/core/src/ptr/const_ptr.rs#L172
> > > + // which is the first version that satisfies `CONFIG_RUSTC_HAS_STABLE_STRICT_PROVENANCE`.
> > > + #[allow(clippy::undocumented_unsafe_blocks)]
> > > + unsafe {
> > > + #[allow(clippy::transmutes_expressible_as_ptr_casts)]
> > > + core::mem::transmute(ptr.cast::<()>())
> > > + }
> >
> > I think we should just use `ptr as usize` here instead. It's going away
> > at some point and it will only affect optimizations (I don't even know
> > if they exist at the moment) of old versions.
>
> Why get cute? I'd rather defer to the standard library.
Ah, this is gone anyway with Boqun's suggestion - this function exists in 1.78.
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-17 14:23 ` [PATCH v5 6/6] rust: use strict provenance APIs Tamir Duberstein
` (2 preceding siblings ...)
2025-03-17 17:50 ` Benno Lossin
@ 2025-03-18 12:29 ` Alice Ryhl
2025-03-18 14:08 ` Tamir Duberstein
2025-03-19 0:23 ` Benno Lossin
3 siblings, 2 replies; 55+ messages in thread
From: Alice Ryhl @ 2025-03-18 12:29 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote:
> Throughout the tree, use the strict provenance APIs stabilized in Rust
> 1.84.0[1]. Retain backwards-compatibility by introducing forwarding
> functions at the `kernel` crate root along with polyfills for rustc <
> 1.84.0.
>
> Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc <
> 1.84.0 as our MSRV is 1.78.0.
>
> In the `kernel` crate, enable the strict provenance lints on rustc >=
> 1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing
> compiler flags that are dependent on the rustc version in use.
>
> Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1]
> Suggested-by: Benno Lossin <benno.lossin@proton.me>
> Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
I'm not convinced that the pros of this change outweigh the cons. I
think this is going to be too confusing for the C developers who look at
this code.
> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> index 719b0a48ff55..96393bcf6bd7 100644
> --- a/rust/kernel/uaccess.rs
> +++ b/rust/kernel/uaccess.rs
> @@ -226,7 +226,9 @@ pub fn read_raw(&mut self, out: &mut [MaybeUninit<u8>]) -> Result {
> }
> // SAFETY: `out_ptr` points into a mutable slice of length `len`, so we may write
> // that many bytes to it.
> - let res = unsafe { bindings::copy_from_user(out_ptr, self.ptr as *const c_void, len) };
> + let res = unsafe {
> + bindings::copy_from_user(out_ptr, crate::with_exposed_provenance(self.ptr), len)
> + };
> if res != 0 {
> return Err(EFAULT);
> }
> @@ -264,7 +266,7 @@ pub fn read<T: FromBytes>(&mut self) -> Result<T> {
> let res = unsafe {
> bindings::_copy_from_user(
> out.as_mut_ptr().cast::<c_void>(),
> - self.ptr as *const c_void,
> + crate::with_exposed_provenance(self.ptr),
> len,
> )
> };
That's especially true for cases like this. These are userspace pointers
that are never dereferenced. It's not useful to care about provenance
here.
Alice
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-18 12:29 ` Alice Ryhl
@ 2025-03-18 14:08 ` Tamir Duberstein
2025-03-19 0:23 ` Benno Lossin
1 sibling, 0 replies; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-18 14:08 UTC (permalink / raw)
To: Alice Ryhl
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Tue, Mar 18, 2025 at 8:29 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote:
> > Throughout the tree, use the strict provenance APIs stabilized in Rust
> > 1.84.0[1]. Retain backwards-compatibility by introducing forwarding
> > functions at the `kernel` crate root along with polyfills for rustc <
> > 1.84.0.
> >
> > Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc <
> > 1.84.0 as our MSRV is 1.78.0.
> >
> > In the `kernel` crate, enable the strict provenance lints on rustc >=
> > 1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing
> > compiler flags that are dependent on the rustc version in use.
> >
> > Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1]
> > Suggested-by: Benno Lossin <benno.lossin@proton.me>
> > Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/
> > Signed-off-by: Tamir Duberstein <tamird@gmail.com>
>
> I'm not convinced that the pros of this change outweigh the cons. I
> think this is going to be too confusing for the C developers who look at
> this code.
>
> > diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> > index 719b0a48ff55..96393bcf6bd7 100644
> > --- a/rust/kernel/uaccess.rs
> > +++ b/rust/kernel/uaccess.rs
> > @@ -226,7 +226,9 @@ pub fn read_raw(&mut self, out: &mut [MaybeUninit<u8>]) -> Result {
> > }
> > // SAFETY: `out_ptr` points into a mutable slice of length `len`, so we may write
> > // that many bytes to it.
> > - let res = unsafe { bindings::copy_from_user(out_ptr, self.ptr as *const c_void, len) };
> > + let res = unsafe {
> > + bindings::copy_from_user(out_ptr, crate::with_exposed_provenance(self.ptr), len)
> > + };
> > if res != 0 {
> > return Err(EFAULT);
> > }
> > @@ -264,7 +266,7 @@ pub fn read<T: FromBytes>(&mut self) -> Result<T> {
> > let res = unsafe {
> > bindings::_copy_from_user(
> > out.as_mut_ptr().cast::<c_void>(),
> > - self.ptr as *const c_void,
> > + crate::with_exposed_provenance(self.ptr),
> > len,
> > )
> > };
>
> That's especially true for cases like this. These are userspace pointers
> that are never dereferenced. It's not useful to care about provenance
> here.
>
> Alice
Let's just drop this last patch. It can be revisited later or not at
all. Perhaps in the future I need to be more willing to say no to
scope creep.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-18 12:29 ` Alice Ryhl
2025-03-18 14:08 ` Tamir Duberstein
@ 2025-03-19 0:23 ` Benno Lossin
2025-03-19 12:21 ` Alice Ryhl
1 sibling, 1 reply; 55+ messages in thread
From: Benno Lossin @ 2025-03-19 0:23 UTC (permalink / raw)
To: Alice Ryhl, Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Tue Mar 18, 2025 at 1:29 PM CET, Alice Ryhl wrote:
> On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote:
>> Throughout the tree, use the strict provenance APIs stabilized in Rust
>> 1.84.0[1]. Retain backwards-compatibility by introducing forwarding
>> functions at the `kernel` crate root along with polyfills for rustc <
>> 1.84.0.
>>
>> Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc <
>> 1.84.0 as our MSRV is 1.78.0.
>>
>> In the `kernel` crate, enable the strict provenance lints on rustc >=
>> 1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing
>> compiler flags that are dependent on the rustc version in use.
>>
>> Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1]
>> Suggested-by: Benno Lossin <benno.lossin@proton.me>
>> Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/
>> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
>
> I'm not convinced that the pros of this change outweigh the cons. I
> think this is going to be too confusing for the C developers who look at
> this code.
1) I think we should eliminate all possible `as` conversions. They are
non-descriptive (since they can do may *very* different things) and
ptr2int conversions are part of that.
2) At some point we will have to move to the provenance API, since
that's what Rust chose to do. I don't think that doing it at a later
point is doing anyone a favor.
3) I don't understand the argument that this is confusing to C devs.
They are just normal functions that are well-documented (and if
that's not the case, we can just improve them upstream). And
functions are much easier to learn about than `as` casts (those are
IMO much more difficult to figure out than then strict provenance
functions).
Thus I think we should keep this patch (with Boqun's improvement).
>> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
>> index 719b0a48ff55..96393bcf6bd7 100644
>> --- a/rust/kernel/uaccess.rs
>> +++ b/rust/kernel/uaccess.rs
>> @@ -226,7 +226,9 @@ pub fn read_raw(&mut self, out: &mut [MaybeUninit<u8>]) -> Result {
>> }
>> // SAFETY: `out_ptr` points into a mutable slice of length `len`, so we may write
>> // that many bytes to it.
>> - let res = unsafe { bindings::copy_from_user(out_ptr, self.ptr as *const c_void, len) };
>> + let res = unsafe {
>> + bindings::copy_from_user(out_ptr, crate::with_exposed_provenance(self.ptr), len)
>> + };
>> if res != 0 {
>> return Err(EFAULT);
>> }
>> @@ -264,7 +266,7 @@ pub fn read<T: FromBytes>(&mut self) -> Result<T> {
>> let res = unsafe {
>> bindings::_copy_from_user(
>> out.as_mut_ptr().cast::<c_void>(),
>> - self.ptr as *const c_void,
>> + crate::with_exposed_provenance(self.ptr),
>> len,
>> )
>> };
>
> That's especially true for cases like this. These are userspace pointers
> that are never dereferenced. It's not useful to care about provenance
> here.
I agree for this case, but I think we shouldn't be using raw pointers
for this to begin with. I'd think that a newtype wrapping `usize` is a
much better fit. It can then also back the `IoRaw` type. AFAIU user
space pointers don't have provenance, right? (if they do, then we should
use this API :)
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-19 0:23 ` Benno Lossin
@ 2025-03-19 12:21 ` Alice Ryhl
2025-03-19 14:14 ` Tamir Duberstein
` (2 more replies)
0 siblings, 3 replies; 55+ messages in thread
From: Alice Ryhl @ 2025-03-19 12:21 UTC (permalink / raw)
To: Benno Lossin
Cc: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan,
linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Wed, Mar 19, 2025 at 12:23:44AM +0000, Benno Lossin wrote:
> On Tue Mar 18, 2025 at 1:29 PM CET, Alice Ryhl wrote:
> > On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote:
> >> Throughout the tree, use the strict provenance APIs stabilized in Rust
> >> 1.84.0[1]. Retain backwards-compatibility by introducing forwarding
> >> functions at the `kernel` crate root along with polyfills for rustc <
> >> 1.84.0.
> >>
> >> Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc <
> >> 1.84.0 as our MSRV is 1.78.0.
> >>
> >> In the `kernel` crate, enable the strict provenance lints on rustc >=
> >> 1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing
> >> compiler flags that are dependent on the rustc version in use.
> >>
> >> Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1]
> >> Suggested-by: Benno Lossin <benno.lossin@proton.me>
> >> Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/
> >> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> >
> > I'm not convinced that the pros of this change outweigh the cons. I
> > think this is going to be too confusing for the C developers who look at
> > this code.
>
> 1) I think we should eliminate all possible `as` conversions. They are
> non-descriptive (since they can do may *very* different things) and
> ptr2int conversions are part of that.
> 2) At some point we will have to move to the provenance API, since
> that's what Rust chose to do. I don't think that doing it at a later
> point is doing anyone a favor.
We don't *have* to do anything. Sure, most `as` conversions can be
removed now that we have fixed the integer type mappings, but I'm still
not convinced by this case.
Like, sure, use it for that one case in `kernel::str` where it uses
integers for pointers for some reason. But most other cases, provenance
isn't useful.
> 3) I don't understand the argument that this is confusing to C devs.
> They are just normal functions that are well-documented (and if
> that's not the case, we can just improve them upstream). And
> functions are much easier to learn about than `as` casts (those are
> IMO much more difficult to figure out than then strict provenance
> functions).
I really don't think that's true, no matter how good the docs are. If
you see `addr as *mut c_void` as a C dev, you are going to immediately
understand what that means. If you see with_exposed_provenance(addr),
you're not going to understand what that means from the name - you have
to interrupt your reading and look up the function with the weird name.
And those docs probably spend a long time talking about stuff that
doesn't matter for your pointer, since it's probably a userspace pointer
or similar.
> Thus I think we should keep this patch (with Boqun's improvement).
>
> >> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> >> index 719b0a48ff55..96393bcf6bd7 100644
> >> --- a/rust/kernel/uaccess.rs
> >> +++ b/rust/kernel/uaccess.rs
> >> @@ -226,7 +226,9 @@ pub fn read_raw(&mut self, out: &mut [MaybeUninit<u8>]) -> Result {
> >> }
> >> // SAFETY: `out_ptr` points into a mutable slice of length `len`, so we may write
> >> // that many bytes to it.
> >> - let res = unsafe { bindings::copy_from_user(out_ptr, self.ptr as *const c_void, len) };
> >> + let res = unsafe {
> >> + bindings::copy_from_user(out_ptr, crate::with_exposed_provenance(self.ptr), len)
> >> + };
> >> if res != 0 {
> >> return Err(EFAULT);
> >> }
> >> @@ -264,7 +266,7 @@ pub fn read<T: FromBytes>(&mut self) -> Result<T> {
> >> let res = unsafe {
> >> bindings::_copy_from_user(
> >> out.as_mut_ptr().cast::<c_void>(),
> >> - self.ptr as *const c_void,
> >> + crate::with_exposed_provenance(self.ptr),
> >> len,
> >> )
> >> };
> >
> > That's especially true for cases like this. These are userspace pointers
> > that are never dereferenced. It's not useful to care about provenance
> > here.
>
> I agree for this case, but I think we shouldn't be using raw pointers
> for this to begin with. I'd think that a newtype wrapping `usize` is a
> much better fit. It can then also back the `IoRaw` type. AFAIU user
> space pointers don't have provenance, right? (if they do, then we should
> use this API :)
We're doing that to the fullest extent possible already. We only convert
them to pointers when calling C FFI functions that take user pointers as
a raw pointer.
Alice
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-19 12:21 ` Alice Ryhl
@ 2025-03-19 14:14 ` Tamir Duberstein
2025-03-19 14:42 ` Benno Lossin
2025-03-19 14:25 ` Benno Lossin
2025-03-19 20:02 ` Benno Lossin
2 siblings, 1 reply; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-19 14:14 UTC (permalink / raw)
To: Alice Ryhl
Cc: Benno Lossin, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan,
linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Wed, Mar 19, 2025 at 8:21 AM Alice Ryhl <aliceryhl@google.com> wrote:
>
> On Wed, Mar 19, 2025 at 12:23:44AM +0000, Benno Lossin wrote:
> > On Tue Mar 18, 2025 at 1:29 PM CET, Alice Ryhl wrote:
> > > On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote:
> > >> Throughout the tree, use the strict provenance APIs stabilized in Rust
> > >> 1.84.0[1]. Retain backwards-compatibility by introducing forwarding
> > >> functions at the `kernel` crate root along with polyfills for rustc <
> > >> 1.84.0.
> > >>
> > >> Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc <
> > >> 1.84.0 as our MSRV is 1.78.0.
> > >>
> > >> In the `kernel` crate, enable the strict provenance lints on rustc >=
> > >> 1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing
> > >> compiler flags that are dependent on the rustc version in use.
> > >>
> > >> Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1]
> > >> Suggested-by: Benno Lossin <benno.lossin@proton.me>
> > >> Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/
> > >> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> > >
> > > I'm not convinced that the pros of this change outweigh the cons. I
> > > think this is going to be too confusing for the C developers who look at
> > > this code.
> >
> > 1) I think we should eliminate all possible `as` conversions. They are
> > non-descriptive (since they can do may *very* different things) and
> > ptr2int conversions are part of that.
> > 2) At some point we will have to move to the provenance API, since
> > that's what Rust chose to do. I don't think that doing it at a later
> > point is doing anyone a favor.
>
> We don't *have* to do anything. Sure, most `as` conversions can be
> removed now that we have fixed the integer type mappings, but I'm still
> not convinced by this case.
>
> Like, sure, use it for that one case in `kernel::str` where it uses
> integers for pointers for some reason. But most other cases, provenance
> isn't useful.
>
> > 3) I don't understand the argument that this is confusing to C devs.
> > They are just normal functions that are well-documented (and if
> > that's not the case, we can just improve them upstream). And
> > functions are much easier to learn about than `as` casts (those are
> > IMO much more difficult to figure out than then strict provenance
> > functions).
>
> I really don't think that's true, no matter how good the docs are. If
> you see `addr as *mut c_void` as a C dev, you are going to immediately
> understand what that means. If you see with_exposed_provenance(addr),
> you're not going to understand what that means from the name - you have
> to interrupt your reading and look up the function with the weird name.
>
> And those docs probably spend a long time talking about stuff that
> doesn't matter for your pointer, since it's probably a userspace pointer
> or similar.
>
> > Thus I think we should keep this patch (with Boqun's improvement).
> >
> > >> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
> > >> index 719b0a48ff55..96393bcf6bd7 100644
> > >> --- a/rust/kernel/uaccess.rs
> > >> +++ b/rust/kernel/uaccess.rs
> > >> @@ -226,7 +226,9 @@ pub fn read_raw(&mut self, out: &mut [MaybeUninit<u8>]) -> Result {
> > >> }
> > >> // SAFETY: `out_ptr` points into a mutable slice of length `len`, so we may write
> > >> // that many bytes to it.
> > >> - let res = unsafe { bindings::copy_from_user(out_ptr, self.ptr as *const c_void, len) };
> > >> + let res = unsafe {
> > >> + bindings::copy_from_user(out_ptr, crate::with_exposed_provenance(self.ptr), len)
> > >> + };
> > >> if res != 0 {
> > >> return Err(EFAULT);
> > >> }
> > >> @@ -264,7 +266,7 @@ pub fn read<T: FromBytes>(&mut self) -> Result<T> {
> > >> let res = unsafe {
> > >> bindings::_copy_from_user(
> > >> out.as_mut_ptr().cast::<c_void>(),
> > >> - self.ptr as *const c_void,
> > >> + crate::with_exposed_provenance(self.ptr),
> > >> len,
> > >> )
> > >> };
> > >
> > > That's especially true for cases like this. These are userspace pointers
> > > that are never dereferenced. It's not useful to care about provenance
> > > here.
> >
> > I agree for this case, but I think we shouldn't be using raw pointers
> > for this to begin with. I'd think that a newtype wrapping `usize` is a
> > much better fit. It can then also back the `IoRaw` type. AFAIU user
> > space pointers don't have provenance, right? (if they do, then we should
> > use this API :)
>
> We're doing that to the fullest extent possible already. We only convert
> them to pointers when calling C FFI functions that take user pointers as
> a raw pointer.
>
> Alice
Personally, I agree with Benno that `as` conversions are a misfeature
in the language.
I think this patch and the ensuing discussion is making perfect the
enemy of good, so I'd prefer to drop it and revisit when the
ergonomics have improved.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-19 14:14 ` Tamir Duberstein
@ 2025-03-19 14:42 ` Benno Lossin
2025-03-19 18:23 ` Tamir Duberstein
0 siblings, 1 reply; 55+ messages in thread
From: Benno Lossin @ 2025-03-19 14:42 UTC (permalink / raw)
To: Tamir Duberstein, Alice Ryhl
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Wed Mar 19, 2025 at 3:14 PM CET, Tamir Duberstein wrote:
> On Wed, Mar 19, 2025 at 8:21 AM Alice Ryhl <aliceryhl@google.com> wrote:
>> On Wed, Mar 19, 2025 at 12:23:44AM +0000, Benno Lossin wrote:
>> > On Tue Mar 18, 2025 at 1:29 PM CET, Alice Ryhl wrote:
>> > > On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote:
>> > >> @@ -264,7 +266,7 @@ pub fn read<T: FromBytes>(&mut self) -> Result<T> {
>> > >> let res = unsafe {
>> > >> bindings::_copy_from_user(
>> > >> out.as_mut_ptr().cast::<c_void>(),
>> > >> - self.ptr as *const c_void,
>> > >> + crate::with_exposed_provenance(self.ptr),
>> > >> len,
>> > >> )
>> > >> };
>> > >
>> > > That's especially true for cases like this. These are userspace pointers
>> > > that are never dereferenced. It's not useful to care about provenance
>> > > here.
>> >
>> > I agree for this case, but I think we shouldn't be using raw pointers
>> > for this to begin with. I'd think that a newtype wrapping `usize` is a
>> > much better fit. It can then also back the `IoRaw` type. AFAIU user
>> > space pointers don't have provenance, right? (if they do, then we should
>> > use this API :)
>>
>> We're doing that to the fullest extent possible already. We only convert
>> them to pointers when calling C FFI functions that take user pointers as
>> a raw pointer.
>>
>> Alice
>
> Personally, I agree with Benno that `as` conversions are a misfeature
> in the language.
>
> I think this patch and the ensuing discussion is making perfect the
> enemy of good, so I'd prefer to drop it and revisit when the
> ergonomics have improved.
I don't think that we need to rush on the rest of the patch series.
Boqun's suggestion is very good and I'm not sure which ergonomics need
to be improved here.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-19 14:42 ` Benno Lossin
@ 2025-03-19 18:23 ` Tamir Duberstein
2025-03-19 20:06 ` Benno Lossin
0 siblings, 1 reply; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-19 18:23 UTC (permalink / raw)
To: Benno Lossin
Cc: Alice Ryhl, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan,
linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Wed, Mar 19, 2025 at 10:42 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Wed Mar 19, 2025 at 3:14 PM CET, Tamir Duberstein wrote:
> > On Wed, Mar 19, 2025 at 8:21 AM Alice Ryhl <aliceryhl@google.com> wrote:
> >> On Wed, Mar 19, 2025 at 12:23:44AM +0000, Benno Lossin wrote:
> >> > On Tue Mar 18, 2025 at 1:29 PM CET, Alice Ryhl wrote:
> >> > > On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote:
> >> > >> @@ -264,7 +266,7 @@ pub fn read<T: FromBytes>(&mut self) -> Result<T> {
> >> > >> let res = unsafe {
> >> > >> bindings::_copy_from_user(
> >> > >> out.as_mut_ptr().cast::<c_void>(),
> >> > >> - self.ptr as *const c_void,
> >> > >> + crate::with_exposed_provenance(self.ptr),
> >> > >> len,
> >> > >> )
> >> > >> };
> >> > >
> >> > > That's especially true for cases like this. These are userspace pointers
> >> > > that are never dereferenced. It's not useful to care about provenance
> >> > > here.
> >> >
> >> > I agree for this case, but I think we shouldn't be using raw pointers
> >> > for this to begin with. I'd think that a newtype wrapping `usize` is a
> >> > much better fit. It can then also back the `IoRaw` type. AFAIU user
> >> > space pointers don't have provenance, right? (if they do, then we should
> >> > use this API :)
> >>
> >> We're doing that to the fullest extent possible already. We only convert
> >> them to pointers when calling C FFI functions that take user pointers as
> >> a raw pointer.
> >>
> >> Alice
> >
> > Personally, I agree with Benno that `as` conversions are a misfeature
> > in the language.
> >
> > I think this patch and the ensuing discussion is making perfect the
> > enemy of good, so I'd prefer to drop it and revisit when the
> > ergonomics have improved.
>
> I don't think that we need to rush on the rest of the patch series.
> Boqun's suggestion is very good and I'm not sure which ergonomics need
> to be improved here.
The improved ergonomics arrive in Rust 1.79. See Boqun's reply that
explains we need to keep all the stubs until then.
Regarding landing the rest of the series - you said it yourself: "it's
only going to get more painful in the long run to change this". The
nature of lints is that the longer you don't enable them, the likelier
you are to have a higher hill to climb later.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-19 18:23 ` Tamir Duberstein
@ 2025-03-19 20:06 ` Benno Lossin
0 siblings, 0 replies; 55+ messages in thread
From: Benno Lossin @ 2025-03-19 20:06 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Alice Ryhl, Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan,
linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Wed Mar 19, 2025 at 7:23 PM CET, Tamir Duberstein wrote:
> The improved ergonomics arrive in Rust 1.79. See Boqun's reply that
> explains we need to keep all the stubs until then.
>
> Regarding landing the rest of the series - you said it yourself: "it's
> only going to get more painful in the long run to change this". The
> nature of lints is that the longer you don't enable them, the likelier
> you are to have a higher hill to climb later.
Yeah that's also true (for some reason I was understanding "dropping"
the patch as "abandoning" the patch :)
As discussed in the meeting, feel free to split the series.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-19 12:21 ` Alice Ryhl
2025-03-19 14:14 ` Tamir Duberstein
@ 2025-03-19 14:25 ` Benno Lossin
2025-03-19 20:02 ` Benno Lossin
2 siblings, 0 replies; 55+ messages in thread
From: Benno Lossin @ 2025-03-19 14:25 UTC (permalink / raw)
To: Alice Ryhl
Cc: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan,
linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Wed Mar 19, 2025 at 1:21 PM CET, Alice Ryhl wrote:
> On Wed, Mar 19, 2025 at 12:23:44AM +0000, Benno Lossin wrote:
>> On Tue Mar 18, 2025 at 1:29 PM CET, Alice Ryhl wrote:
>> > On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote:
>> >> Throughout the tree, use the strict provenance APIs stabilized in Rust
>> >> 1.84.0[1]. Retain backwards-compatibility by introducing forwarding
>> >> functions at the `kernel` crate root along with polyfills for rustc <
>> >> 1.84.0.
>> >>
>> >> Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc <
>> >> 1.84.0 as our MSRV is 1.78.0.
>> >>
>> >> In the `kernel` crate, enable the strict provenance lints on rustc >=
>> >> 1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing
>> >> compiler flags that are dependent on the rustc version in use.
>> >>
>> >> Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1]
>> >> Suggested-by: Benno Lossin <benno.lossin@proton.me>
>> >> Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/
>> >> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
>> >
>> > I'm not convinced that the pros of this change outweigh the cons. I
>> > think this is going to be too confusing for the C developers who look at
>> > this code.
>>
>> 1) I think we should eliminate all possible `as` conversions. They are
>> non-descriptive (since they can do may *very* different things) and
>> ptr2int conversions are part of that.
>> 2) At some point we will have to move to the provenance API, since
>> that's what Rust chose to do. I don't think that doing it at a later
>> point is doing anyone a favor.
>
> We don't *have* to do anything. Sure, most `as` conversions can be
> removed now that we have fixed the integer type mappings, but I'm still
> not convinced by this case.
>
> Like, sure, use it for that one case in `kernel::str` where it uses
> integers for pointers for some reason. But most other cases, provenance
> isn't useful.
I disagree, it's only going to get more painful in the long run to
change this.
>> 3) I don't understand the argument that this is confusing to C devs.
>> They are just normal functions that are well-documented (and if
>> that's not the case, we can just improve them upstream). And
>> functions are much easier to learn about than `as` casts (those are
>> IMO much more difficult to figure out than then strict provenance
>> functions).
>
> I really don't think that's true, no matter how good the docs are. If
> you see `addr as *mut c_void` as a C dev, you are going to immediately
> understand what that means. If you see with_exposed_provenance(addr),
> you're not going to understand what that means from the name - you have
> to interrupt your reading and look up the function with the weird name.
I see this as a double edged sword, yes `addr as *mut c_void` might seem
more easily digestible on the first encounter, but that might also lead
them to never look up what it exactly does.
And I don't think that we should optimize these functions for C readers.
They aren't used commonly (or supposed to IMO) and there are several
other functions that are similarly confusing if not more already in our
codebase.
> And those docs probably spend a long time talking about stuff that
> doesn't matter for your pointer, since it's probably a userspace pointer
> or similar.
For userspace pointers, see below.
>> Thus I think we should keep this patch (with Boqun's improvement).
>>
>> >> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
>> >> index 719b0a48ff55..96393bcf6bd7 100644
>> >> --- a/rust/kernel/uaccess.rs
>> >> +++ b/rust/kernel/uaccess.rs
>> >> @@ -226,7 +226,9 @@ pub fn read_raw(&mut self, out: &mut [MaybeUninit<u8>]) -> Result {
>> >> }
>> >> // SAFETY: `out_ptr` points into a mutable slice of length `len`, so we may write
>> >> // that many bytes to it.
>> >> - let res = unsafe { bindings::copy_from_user(out_ptr, self.ptr as *const c_void, len) };
>> >> + let res = unsafe {
>> >> + bindings::copy_from_user(out_ptr, crate::with_exposed_provenance(self.ptr), len)
>> >> + };
>> >> if res != 0 {
>> >> return Err(EFAULT);
>> >> }
>> >> @@ -264,7 +266,7 @@ pub fn read<T: FromBytes>(&mut self) -> Result<T> {
>> >> let res = unsafe {
>> >> bindings::_copy_from_user(
>> >> out.as_mut_ptr().cast::<c_void>(),
>> >> - self.ptr as *const c_void,
>> >> + crate::with_exposed_provenance(self.ptr),
>> >> len,
>> >> )
>> >> };
>> >
>> > That's especially true for cases like this. These are userspace pointers
>> > that are never dereferenced. It's not useful to care about provenance
>> > here.
>>
>> I agree for this case, but I think we shouldn't be using raw pointers
>> for this to begin with. I'd think that a newtype wrapping `usize` is a
>> much better fit. It can then also back the `IoRaw` type. AFAIU user
>> space pointers don't have provenance, right? (if they do, then we should
>> use this API :)
>
> We're doing that to the fullest extent possible already. We only convert
> them to pointers when calling C FFI functions that take user pointers as
> a raw pointer.
We should make bindgen use that type in those interfaces already.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 6/6] rust: use strict provenance APIs
2025-03-19 12:21 ` Alice Ryhl
2025-03-19 14:14 ` Tamir Duberstein
2025-03-19 14:25 ` Benno Lossin
@ 2025-03-19 20:02 ` Benno Lossin
2 siblings, 0 replies; 55+ messages in thread
From: Benno Lossin @ 2025-03-19 20:02 UTC (permalink / raw)
To: Alice Ryhl
Cc: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan,
linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Wed Mar 19, 2025 at 1:21 PM CET, Alice Ryhl wrote:
> On Wed, Mar 19, 2025 at 12:23:44AM +0000, Benno Lossin wrote:
>> On Tue Mar 18, 2025 at 1:29 PM CET, Alice Ryhl wrote:
>> > On Mon, Mar 17, 2025 at 10:23:56AM -0400, Tamir Duberstein wrote:
>> >> Throughout the tree, use the strict provenance APIs stabilized in Rust
>> >> 1.84.0[1]. Retain backwards-compatibility by introducing forwarding
>> >> functions at the `kernel` crate root along with polyfills for rustc <
>> >> 1.84.0.
>> >>
>> >> Use `#[allow(clippy::incompatible_msrv)]` to avoid warnings on rustc <
>> >> 1.84.0 as our MSRV is 1.78.0.
>> >>
>> >> In the `kernel` crate, enable the strict provenance lints on rustc >=
>> >> 1.84.0; do this in `lib.rs` rather than `Makefile` to avoid introducing
>> >> compiler flags that are dependent on the rustc version in use.
>> >>
>> >> Link: https://blog.rust-lang.org/2025/01/09/Rust-1.84.0.html#strict-provenance-apis [1]
>> >> Suggested-by: Benno Lossin <benno.lossin@proton.me>
>> >> Link: https://lore.kernel.org/all/D8EIXDMRXMJP.36TFCGWZBRS3Y@proton.me/
>> >> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
>> >
>> > I'm not convinced that the pros of this change outweigh the cons. I
>> > think this is going to be too confusing for the C developers who look at
>> > this code.
>>
>> 1) I think we should eliminate all possible `as` conversions. They are
>> non-descriptive (since they can do may *very* different things) and
>> ptr2int conversions are part of that.
>> 2) At some point we will have to move to the provenance API, since
>> that's what Rust chose to do. I don't think that doing it at a later
>> point is doing anyone a favor.
>
> We don't *have* to do anything. Sure, most `as` conversions can be
> removed now that we have fixed the integer type mappings, but I'm still
> not convinced by this case.
>
> Like, sure, use it for that one case in `kernel::str` where it uses
> integers for pointers for some reason. But most other cases, provenance
> isn't useful.
>
>> 3) I don't understand the argument that this is confusing to C devs.
>> They are just normal functions that are well-documented (and if
>> that's not the case, we can just improve them upstream). And
>> functions are much easier to learn about than `as` casts (those are
>> IMO much more difficult to figure out than then strict provenance
>> functions).
>
> I really don't think that's true, no matter how good the docs are. If
> you see `addr as *mut c_void` as a C dev, you are going to immediately
> understand what that means. If you see with_exposed_provenance(addr),
> you're not going to understand what that means from the name - you have
> to interrupt your reading and look up the function with the weird name.
>
> And those docs probably spend a long time talking about stuff that
> doesn't matter for your pointer, since it's probably a userspace pointer
> or similar.
>
>> Thus I think we should keep this patch (with Boqun's improvement).
>>
>> >> diff --git a/rust/kernel/uaccess.rs b/rust/kernel/uaccess.rs
>> >> index 719b0a48ff55..96393bcf6bd7 100644
>> >> --- a/rust/kernel/uaccess.rs
>> >> +++ b/rust/kernel/uaccess.rs
>> >> @@ -226,7 +226,9 @@ pub fn read_raw(&mut self, out: &mut [MaybeUninit<u8>]) -> Result {
>> >> }
>> >> // SAFETY: `out_ptr` points into a mutable slice of length `len`, so we may write
>> >> // that many bytes to it.
>> >> - let res = unsafe { bindings::copy_from_user(out_ptr, self.ptr as *const c_void, len) };
>> >> + let res = unsafe {
>> >> + bindings::copy_from_user(out_ptr, crate::with_exposed_provenance(self.ptr), len)
>> >> + };
>> >> if res != 0 {
>> >> return Err(EFAULT);
>> >> }
>> >> @@ -264,7 +266,7 @@ pub fn read<T: FromBytes>(&mut self) -> Result<T> {
>> >> let res = unsafe {
>> >> bindings::_copy_from_user(
>> >> out.as_mut_ptr().cast::<c_void>(),
>> >> - self.ptr as *const c_void,
>> >> + crate::with_exposed_provenance(self.ptr),
>> >> len,
>> >> )
>> >> };
>> >
>> > That's especially true for cases like this. These are userspace pointers
>> > that are never dereferenced. It's not useful to care about provenance
>> > here.
>>
>> I agree for this case, but I think we shouldn't be using raw pointers
>> for this to begin with. I'd think that a newtype wrapping `usize` is a
>> much better fit. It can then also back the `IoRaw` type. AFAIU user
>> space pointers don't have provenance, right? (if they do, then we should
>> use this API :)
>
> We're doing that to the fullest extent possible already. We only convert
> them to pointers when calling C FFI functions that take user pointers as
> a raw pointer.
In our meeting, we discussed all of this in more detail. I now
understand Alice's concern better: it's about userspace pointers, they
don't have provenance and thus shouldn't use the strict provenance API.
There are two possible solutions to this:
* introduce a transparent `UserPtr` type that bindgen uses instead of a
raw pointer when it encounters a userspace pointer (annotated on the C
side).
* use a `to_user_pointer` function instead of `without_provenance` to
better convey the meaning.
I prefer the first one, but it probably needs special bindgen support,
so we should go with the second one until we can change it.
Miguel also said that he wants to have a piece of documentation in the
patch. I can write that, if he specifies a bit more what exactly it
should contain :)
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 0/6] rust: reduce pointer casts, enable related lints
2025-03-17 14:23 [PATCH v5 0/6] rust: reduce pointer casts, enable related lints Tamir Duberstein
` (5 preceding siblings ...)
2025-03-17 14:23 ` [PATCH v5 6/6] rust: use strict provenance APIs Tamir Duberstein
@ 2025-03-19 20:20 ` Tamir Duberstein
2025-03-24 20:16 ` Benno Lossin
7 siblings, 0 replies; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-19 20:20 UTC (permalink / raw)
To: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Benno Lossin, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Mon, Mar 17, 2025 at 10:23 AM Tamir Duberstein <tamird@gmail.com> wrote:
>
> This started with a patch that enabled `clippy::ptr_as_ptr`. Benno
> Lossin suggested I also look into `clippy::ptr_cast_constness` and I
> discovered `clippy::as_ptr_cast_mut`. This series now enables all 3
> lints. It also enables `clippy::as_underscore` which ensures other
> pointer casts weren't missed. The first commit reduces the need for
> pointer casts and is shared with another series[1].
>
> The final patch also enables pointer provenance lints and fixes
> violations. See that commit message for details. The build system
> portion of that commit is pretty messy but I couldn't find a better way
> to convincingly ensure that these lints were applied globally.
> Suggestions would be very welcome.
>
> Link: https://lore.kernel.org/all/20250307-no-offset-v1-0-0c728f63b69c@gmail.com/ [1]
>
> Signed-off-by: Tamir Duberstein <tamird@gmail.com>
> ---
> Changes in v5:
> - Use `pointer::addr` in OF. (Boqun Feng)
> - Add documentation on stubs. (Benno Lossin)
> - Mark stubs `#[inline]`.
> - Pick up Alice's RB on a shared commit from
> https://lore.kernel.org/all/Z9f-3Aj3_FWBZRrm@google.com/.
> - Link to v4: https://lore.kernel.org/r/20250315-ptr-as-ptr-v4-0-b2d72c14dc26@gmail.com
>
> Changes in v4:
> - Add missing SoB. (Benno Lossin)
> - Use `without_provenance_mut` in alloc. (Boqun Feng)
> - Limit strict provenance lints to the `kernel` crate to avoid complex
> logic in the build system. This can be revisited on MSRV >= 1.84.0.
> - Rebase on rust-next.
> - Link to v3: https://lore.kernel.org/r/20250314-ptr-as-ptr-v3-0-e7ba61048f4a@gmail.com
>
> Changes in v3:
> - Fixed clippy warning in rust/kernel/firmware.rs. (kernel test robot)
> Link: https://lore.kernel.org/all/202503120332.YTCpFEvv-lkp@intel.com/
> - s/as u64/as bindings::phys_addr_t/g. (Benno Lossin)
> - Use strict provenance APIs and enable lints. (Benno Lossin)
> - Link to v2: https://lore.kernel.org/r/20250309-ptr-as-ptr-v2-0-25d60ad922b7@gmail.com
>
> Changes in v2:
> - Fixed typo in first commit message.
> - Added additional patches, converted to series.
> - Link to v1: https://lore.kernel.org/r/20250307-ptr-as-ptr-v1-1-582d06514c98@gmail.com
>
> ---
> Tamir Duberstein (6):
> rust: retain pointer mut-ness in `container_of!`
> rust: enable `clippy::ptr_as_ptr` lint
> rust: enable `clippy::ptr_cast_constness` lint
> rust: enable `clippy::as_ptr_cast_mut` lint
> rust: enable `clippy::as_underscore` lint
> rust: use strict provenance APIs
>
> Makefile | 4 ++
> init/Kconfig | 3 +
> rust/bindings/lib.rs | 1 +
> rust/kernel/alloc.rs | 2 +-
> rust/kernel/alloc/allocator_test.rs | 2 +-
> rust/kernel/alloc/kvec.rs | 4 +-
> rust/kernel/block/mq/operations.rs | 2 +-
> rust/kernel/block/mq/request.rs | 7 +-
> rust/kernel/device.rs | 5 +-
> rust/kernel/device_id.rs | 2 +-
> rust/kernel/devres.rs | 19 +++---
> rust/kernel/error.rs | 2 +-
> rust/kernel/firmware.rs | 3 +-
> rust/kernel/fs/file.rs | 2 +-
> rust/kernel/io.rs | 16 ++---
> rust/kernel/kunit.rs | 15 ++---
> rust/kernel/lib.rs | 113 ++++++++++++++++++++++++++++++++-
> rust/kernel/list/impl_list_item_mod.rs | 2 +-
> rust/kernel/miscdevice.rs | 2 +-
> rust/kernel/of.rs | 6 +-
> rust/kernel/pci.rs | 15 +++--
> rust/kernel/platform.rs | 6 +-
> rust/kernel/print.rs | 11 ++--
> rust/kernel/rbtree.rs | 23 +++----
> rust/kernel/seq_file.rs | 3 +-
> rust/kernel/str.rs | 18 ++----
> rust/kernel/sync/poll.rs | 2 +-
> rust/kernel/uaccess.rs | 12 ++--
> rust/kernel/workqueue.rs | 12 ++--
> rust/uapi/lib.rs | 1 +
> 30 files changed, 218 insertions(+), 97 deletions(-)
> ---
> base-commit: 498f7ee4773f22924f00630136da8575f38954e8
> change-id: 20250307-ptr-as-ptr-21b1867fc4d4
Per the discussion in today's Rust-for-Linux meeting and Benno's
reply[0] please take this series without the last patch, whenever you
see fit to do so. At this time no changes have been requested to the
first 5 patches, so I am not planning to respin.
Cheers.
Tamir
[0] https://lore.kernel.org/all/D8KIHNXCPE0P.K4MD7QJ1AC17@proton.me/
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 0/6] rust: reduce pointer casts, enable related lints
2025-03-17 14:23 [PATCH v5 0/6] rust: reduce pointer casts, enable related lints Tamir Duberstein
` (6 preceding siblings ...)
2025-03-19 20:20 ` [PATCH v5 0/6] rust: reduce pointer casts, enable related lints Tamir Duberstein
@ 2025-03-24 20:16 ` Benno Lossin
2025-03-24 20:55 ` Tamir Duberstein
7 siblings, 1 reply; 55+ messages in thread
From: Benno Lossin @ 2025-03-24 20:16 UTC (permalink / raw)
To: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan
Cc: linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Mon Mar 17, 2025 at 3:23 PM CET, Tamir Duberstein wrote:
> This started with a patch that enabled `clippy::ptr_as_ptr`. Benno
> Lossin suggested I also look into `clippy::ptr_cast_constness` and I
> discovered `clippy::as_ptr_cast_mut`. This series now enables all 3
> lints. It also enables `clippy::as_underscore` which ensures other
> pointer casts weren't missed. The first commit reduces the need for
> pointer casts and is shared with another series[1].
>
> The final patch also enables pointer provenance lints and fixes
> violations. See that commit message for details. The build system
> portion of that commit is pretty messy but I couldn't find a better way
> to convincingly ensure that these lints were applied globally.
> Suggestions would be very welcome.
I applied the patches to v6.14-rc7 and did a quick pass with
rg -nC 3 -t rust ' as ' | bat -l rust
to see if there are any cases left that we could fix and I found a
couple:
* there are several cases of `number as int_type` (like `num as c_int`
or `my_u32 as usize` etc.) not sure what we can do about these, some
are probably unavoidable, but since the kernel doesn't support 16 bit
systems (that is true, right?), we *could* have a `From<u32> for
usize` impl...
* some instances of `'|' as u32` (samples/rust/rust_misc_device.rs:112).
There is a `From<char> for u32` impl, so this can just be replaced
with `.into()` (or maybe by using a byte literal `b'|'`?).
* `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
replace with `let ptr: *const ... = shared_ref;`. Don't know if there
is a clippy lint for this.
* some pointer casts in rust/kernel/list/impl_list_item_mod.rs:{253,254}
not sure if they can be converted though (maybe they are unsizing the
pointer?)
Another pointer cast in rust/kernel/driver.rs:81 (I'm pretty sure this
one can be replaced by a `.cast()`)
Some clippy lints that we could also enable that share the spirit of
this series:
* `char_lit_as_u8` (maybe that also covers the `'|' as u32` case from
above?)
* `cast_lossless` (maybe this catches some of the `num as int_type`
conversions I mentioned above)
I'll leave it up to you what you want to do with this: add it to this
series, make a new one, or let someone else handle it. If you don't want
to handle it, let me know, then I'll create a good-first-issue :)
> ---
> Tamir Duberstein (6):
> rust: retain pointer mut-ness in `container_of!`
> rust: enable `clippy::ptr_as_ptr` lint
> rust: enable `clippy::ptr_cast_constness` lint
> rust: enable `clippy::as_ptr_cast_mut` lint
> rust: enable `clippy::as_underscore` lint
> rust: use strict provenance APIs
>
> Makefile | 4 ++
> init/Kconfig | 3 +
> rust/bindings/lib.rs | 1 +
> rust/kernel/alloc.rs | 2 +-
> rust/kernel/alloc/allocator_test.rs | 2 +-
> rust/kernel/alloc/kvec.rs | 4 +-
> rust/kernel/block/mq/operations.rs | 2 +-
> rust/kernel/block/mq/request.rs | 7 +-
> rust/kernel/device.rs | 5 +-
> rust/kernel/device_id.rs | 2 +-
> rust/kernel/devres.rs | 19 +++---
> rust/kernel/error.rs | 2 +-
> rust/kernel/firmware.rs | 3 +-
> rust/kernel/fs/file.rs | 2 +-
> rust/kernel/io.rs | 16 ++---
> rust/kernel/kunit.rs | 15 ++---
> rust/kernel/lib.rs | 113 ++++++++++++++++++++++++++++++++-
> rust/kernel/list/impl_list_item_mod.rs | 2 +-
> rust/kernel/miscdevice.rs | 2 +-
> rust/kernel/of.rs | 6 +-
> rust/kernel/pci.rs | 15 +++--
> rust/kernel/platform.rs | 6 +-
> rust/kernel/print.rs | 11 ++--
> rust/kernel/rbtree.rs | 23 +++----
> rust/kernel/seq_file.rs | 3 +-
> rust/kernel/str.rs | 18 ++----
> rust/kernel/sync/poll.rs | 2 +-
> rust/kernel/uaccess.rs | 12 ++--
> rust/kernel/workqueue.rs | 12 ++--
> rust/uapi/lib.rs | 1 +
> 30 files changed, 218 insertions(+), 97 deletions(-)
> ---
> base-commit: 498f7ee4773f22924f00630136da8575f38954e8
Btw I didn't find this commit anywhere I usually check, where is it
from?
---
Cheers,
Benno
> change-id: 20250307-ptr-as-ptr-21b1867fc4d4
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 0/6] rust: reduce pointer casts, enable related lints
2025-03-24 20:16 ` Benno Lossin
@ 2025-03-24 20:55 ` Tamir Duberstein
2025-03-24 21:16 ` Tamir Duberstein
` (2 more replies)
0 siblings, 3 replies; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-24 20:55 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Mon Mar 17, 2025 at 3:23 PM CET, Tamir Duberstein wrote:
> > This started with a patch that enabled `clippy::ptr_as_ptr`. Benno
> > Lossin suggested I also look into `clippy::ptr_cast_constness` and I
> > discovered `clippy::as_ptr_cast_mut`. This series now enables all 3
> > lints. It also enables `clippy::as_underscore` which ensures other
> > pointer casts weren't missed. The first commit reduces the need for
> > pointer casts and is shared with another series[1].
> >
> > The final patch also enables pointer provenance lints and fixes
> > violations. See that commit message for details. The build system
> > portion of that commit is pretty messy but I couldn't find a better way
> > to convincingly ensure that these lints were applied globally.
> > Suggestions would be very welcome.
>
> I applied the patches to v6.14-rc7 and did a quick pass with
>
> rg -nC 3 -t rust ' as ' | bat -l rust
>
> to see if there are any cases left that we could fix and I found a
> couple:
>
> * there are several cases of `number as int_type` (like `num as c_int`
> or `my_u32 as usize` etc.) not sure what we can do about these, some
> are probably unavoidable, but since the kernel doesn't support 16 bit
> systems (that is true, right?), we *could* have a `From<u32> for
> usize` impl...
Yeah, these are the most difficult ones to get rid of.
> * some instances of `'|' as u32` (samples/rust/rust_misc_device.rs:112).
> There is a `From<char> for u32` impl, so this can just be replaced
> with `.into()` (or maybe by using a byte literal `b'|'`?).
We can enable https://rust-lang.github.io/rust-clippy/master/index.html?levels=allow#cast_lossless
for this one.
> * `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
> rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
> replace with `let ptr: *const ... = shared_ref;`. Don't know if there
> is a clippy lint for this.
I think there's not a focused one. There's a nuclear option:
https://rust-lang.github.io/rust-clippy/master/index.html?levels=allow#as_conversions
> * some pointer casts in rust/kernel/list/impl_list_item_mod.rs:{253,254}
> not sure if they can be converted though (maybe they are unsizing the
> pointer?)
I have a local series that gets rid of these by doing similar things
to https://lore.kernel.org/all/20250307-no-offset-v1-0-0c728f63b69c@gmail.com/.
I can send it later this week but it probably can't land until Alice
is back from vacation; she was the author of this code.
> Another pointer cast in rust/kernel/driver.rs:81 (I'm pretty sure this
> one can be replaced by a `.cast()`)
>
> Some clippy lints that we could also enable that share the spirit of
> this series:
>
> * `char_lit_as_u8` (maybe that also covers the `'|' as u32` case from
> above?)
It's already enabled, it's warn-by-default.
> * `cast_lossless` (maybe this catches some of the `num as int_type`
> conversions I mentioned above)
Yeah, suggested the same above. I had hoped this would deal with the
char as u32 pattern but it did not.
> I'll leave it up to you what you want to do with this: add it to this
> series, make a new one, or let someone else handle it. If you don't want
> to handle it, let me know, then I'll create a good-first-issue :)
I'll add a patch for `cast_lossless` -- the rest should probably go
into an issue.
>
> > ---
> > Tamir Duberstein (6):
> > rust: retain pointer mut-ness in `container_of!`
> > rust: enable `clippy::ptr_as_ptr` lint
> > rust: enable `clippy::ptr_cast_constness` lint
> > rust: enable `clippy::as_ptr_cast_mut` lint
> > rust: enable `clippy::as_underscore` lint
> > rust: use strict provenance APIs
> >
> > Makefile | 4 ++
> > init/Kconfig | 3 +
> > rust/bindings/lib.rs | 1 +
> > rust/kernel/alloc.rs | 2 +-
> > rust/kernel/alloc/allocator_test.rs | 2 +-
> > rust/kernel/alloc/kvec.rs | 4 +-
> > rust/kernel/block/mq/operations.rs | 2 +-
> > rust/kernel/block/mq/request.rs | 7 +-
> > rust/kernel/device.rs | 5 +-
> > rust/kernel/device_id.rs | 2 +-
> > rust/kernel/devres.rs | 19 +++---
> > rust/kernel/error.rs | 2 +-
> > rust/kernel/firmware.rs | 3 +-
> > rust/kernel/fs/file.rs | 2 +-
> > rust/kernel/io.rs | 16 ++---
> > rust/kernel/kunit.rs | 15 ++---
> > rust/kernel/lib.rs | 113 ++++++++++++++++++++++++++++++++-
> > rust/kernel/list/impl_list_item_mod.rs | 2 +-
> > rust/kernel/miscdevice.rs | 2 +-
> > rust/kernel/of.rs | 6 +-
> > rust/kernel/pci.rs | 15 +++--
> > rust/kernel/platform.rs | 6 +-
> > rust/kernel/print.rs | 11 ++--
> > rust/kernel/rbtree.rs | 23 +++----
> > rust/kernel/seq_file.rs | 3 +-
> > rust/kernel/str.rs | 18 ++----
> > rust/kernel/sync/poll.rs | 2 +-
> > rust/kernel/uaccess.rs | 12 ++--
> > rust/kernel/workqueue.rs | 12 ++--
> > rust/uapi/lib.rs | 1 +
> > 30 files changed, 218 insertions(+), 97 deletions(-)
> > ---
> > base-commit: 498f7ee4773f22924f00630136da8575f38954e8
>
> Btw I didn't find this commit anywhere I usually check, where is it
> from?
It was probably nowhere, a local frankenstein that included some fixes
from rust-fixes.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 0/6] rust: reduce pointer casts, enable related lints
2025-03-24 20:55 ` Tamir Duberstein
@ 2025-03-24 21:16 ` Tamir Duberstein
2025-03-24 21:39 ` Benno Lossin
2025-03-24 21:35 ` Tamir Duberstein
2025-03-24 21:55 ` Benno Lossin
2 siblings, 1 reply; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-24 21:16 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 24, 2025 at 4:55 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >
> > On Mon Mar 17, 2025 at 3:23 PM CET, Tamir Duberstein wrote:
> > > This started with a patch that enabled `clippy::ptr_as_ptr`. Benno
> > > Lossin suggested I also look into `clippy::ptr_cast_constness` and I
> > > discovered `clippy::as_ptr_cast_mut`. This series now enables all 3
> > > lints. It also enables `clippy::as_underscore` which ensures other
> > > pointer casts weren't missed. The first commit reduces the need for
> > > pointer casts and is shared with another series[1].
> > >
> > > The final patch also enables pointer provenance lints and fixes
> > > violations. See that commit message for details. The build system
> > > portion of that commit is pretty messy but I couldn't find a better way
> > > to convincingly ensure that these lints were applied globally.
> > > Suggestions would be very welcome.
> >
> > I applied the patches to v6.14-rc7 and did a quick pass with
So I rebased this on rust-next and fixed a few more instances (in
addition to enabling the extra lint), but I realized that rust-next is
still based on v6.14-rc5. I think we're going to have the same problem
here as in the &raw series; either Miguel is going to have to apply
fixups when picking these patches, or we have to split the fixes out
from the lints and land it over several cycles. Thoughts?
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 0/6] rust: reduce pointer casts, enable related lints
2025-03-24 21:16 ` Tamir Duberstein
@ 2025-03-24 21:39 ` Benno Lossin
0 siblings, 0 replies; 55+ messages in thread
From: Benno Lossin @ 2025-03-24 21:39 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon Mar 24, 2025 at 10:16 PM CET, Tamir Duberstein wrote:
> On Mon, Mar 24, 2025 at 4:55 PM Tamir Duberstein <tamird@gmail.com> wrote:
>> On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> > On Mon Mar 17, 2025 at 3:23 PM CET, Tamir Duberstein wrote:
>> > > This started with a patch that enabled `clippy::ptr_as_ptr`. Benno
>> > > Lossin suggested I also look into `clippy::ptr_cast_constness` and I
>> > > discovered `clippy::as_ptr_cast_mut`. This series now enables all 3
>> > > lints. It also enables `clippy::as_underscore` which ensures other
>> > > pointer casts weren't missed. The first commit reduces the need for
>> > > pointer casts and is shared with another series[1].
>> > >
>> > > The final patch also enables pointer provenance lints and fixes
>> > > violations. See that commit message for details. The build system
>> > > portion of that commit is pretty messy but I couldn't find a better way
>> > > to convincingly ensure that these lints were applied globally.
>> > > Suggestions would be very welcome.
>> >
>> > I applied the patches to v6.14-rc7 and did a quick pass with
>
> So I rebased this on rust-next and fixed a few more instances (in
> addition to enabling the extra lint), but I realized that rust-next is
> still based on v6.14-rc5. I think we're going to have the same problem
> here as in the &raw series; either Miguel is going to have to apply
> fixups when picking these patches, or we have to split the fixes out
> from the lints and land it over several cycles. Thoughts?
One option is to pick the patches early in the cycle and then ask
authors of patches to rebase. We did that with the alloc changes in the
past.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 0/6] rust: reduce pointer casts, enable related lints
2025-03-24 20:55 ` Tamir Duberstein
2025-03-24 21:16 ` Tamir Duberstein
@ 2025-03-24 21:35 ` Tamir Duberstein
2025-03-24 21:55 ` Benno Lossin
2 siblings, 0 replies; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-24 21:35 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 24, 2025 at 4:55 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >
>
> > * some pointer casts in rust/kernel/list/impl_list_item_mod.rs:{253,254}
> > not sure if they can be converted though (maybe they are unsizing the
> > pointer?)
>
> I have a local series that gets rid of these by doing similar things
> to https://lore.kernel.org/all/20250307-no-offset-v1-0-0c728f63b69c@gmail.com/.
> I can send it later this week but it probably can't land until Alice
> is back from vacation; she was the author of this code.
https://lore.kernel.org/all/20250324-list-no-offset-v1-0-afd2b7fc442a@gmail.com/
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 0/6] rust: reduce pointer casts, enable related lints
2025-03-24 20:55 ` Tamir Duberstein
2025-03-24 21:16 ` Tamir Duberstein
2025-03-24 21:35 ` Tamir Duberstein
@ 2025-03-24 21:55 ` Benno Lossin
2025-03-24 21:59 ` Tamir Duberstein
2 siblings, 1 reply; 55+ messages in thread
From: Benno Lossin @ 2025-03-24 21:55 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon Mar 24, 2025 at 9:55 PM CET, Tamir Duberstein wrote:
> On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> * `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
>> rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
>> replace with `let ptr: *const ... = shared_ref;`. Don't know if there
>> is a clippy lint for this.
>
> I think there's not a focused one. There's a nuclear option:
> https://rust-lang.github.io/rust-clippy/master/index.html?levels=allow#as_conversions
Yeah I saw that one, I don't think it's a good idea, since there will be
false positives.
>> * some pointer casts in rust/kernel/list/impl_list_item_mod.rs:{253,254}
>> not sure if they can be converted though (maybe they are unsizing the
>> pointer?)
>
> I have a local series that gets rid of these by doing similar things
> to https://lore.kernel.org/all/20250307-no-offset-v1-0-0c728f63b69c@gmail.com/.
> I can send it later this week but it probably can't land until Alice
> is back from vacation; she was the author of this code.
No worries, as I wrote below, I think it's fine to do that in a new
series.
>> Another pointer cast in rust/kernel/driver.rs:81 (I'm pretty sure this
>> one can be replaced by a `.cast()`)
>>
>> Some clippy lints that we could also enable that share the spirit of
>> this series:
>>
>> * `char_lit_as_u8` (maybe that also covers the `'|' as u32` case from
>> above?)
>
> It's already enabled, it's warn-by-default.
Ah I see, didn't look :)
>> * `cast_lossless` (maybe this catches some of the `num as int_type`
>> conversions I mentioned above)
>
> Yeah, suggested the same above. I had hoped this would deal with the
> char as u32 pattern but it did not.
Aw that's a shame. Maybe we should create a clippy issue for that,
thoughts?
>> I'll leave it up to you what you want to do with this: add it to this
>> series, make a new one, or let someone else handle it. If you don't want
>> to handle it, let me know, then I'll create a good-first-issue :)
>
> I'll add a patch for `cast_lossless` -- the rest should probably go
> into an issue.
Do you mind filing the issue? Then you can decide yourself what you want
to do yourself vs what you want to leave for others. Feel free to copy
from my mail summary.
Also I wouldn't mark it as a good-first-issue yet, since it's pretty
complicated and needs to be delayed/based on this series.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 0/6] rust: reduce pointer casts, enable related lints
2025-03-24 21:55 ` Benno Lossin
@ 2025-03-24 21:59 ` Tamir Duberstein
2025-03-25 11:05 ` Benno Lossin
0 siblings, 1 reply; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-24 21:59 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon, Mar 24, 2025 at 5:55 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Mon Mar 24, 2025 at 9:55 PM CET, Tamir Duberstein wrote:
> > On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >> * `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
> >> rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
> >> replace with `let ptr: *const ... = shared_ref;`. Don't know if there
> >> is a clippy lint for this.
> >
> > I think there's not a focused one. There's a nuclear option:
> > https://rust-lang.github.io/rust-clippy/master/index.html?levels=allow#as_conversions
>
> Yeah I saw that one, I don't think it's a good idea, since there will be
> false positives.
>
> >> * some pointer casts in rust/kernel/list/impl_list_item_mod.rs:{253,254}
> >> not sure if they can be converted though (maybe they are unsizing the
> >> pointer?)
> >
> > I have a local series that gets rid of these by doing similar things
> > to https://lore.kernel.org/all/20250307-no-offset-v1-0-0c728f63b69c@gmail.com/.
> > I can send it later this week but it probably can't land until Alice
> > is back from vacation; she was the author of this code.
>
> No worries, as I wrote below, I think it's fine to do that in a new
> series.
>
> >> Another pointer cast in rust/kernel/driver.rs:81 (I'm pretty sure this
> >> one can be replaced by a `.cast()`)
> >>
> >> Some clippy lints that we could also enable that share the spirit of
> >> this series:
> >>
> >> * `char_lit_as_u8` (maybe that also covers the `'|' as u32` case from
> >> above?)
> >
> > It's already enabled, it's warn-by-default.
>
> Ah I see, didn't look :)
>
> >> * `cast_lossless` (maybe this catches some of the `num as int_type`
> >> conversions I mentioned above)
> >
> > Yeah, suggested the same above. I had hoped this would deal with the
> > char as u32 pattern but it did not.
>
> Aw that's a shame. Maybe we should create a clippy issue for that,
> thoughts?
Yeah, it's not clear to me why it isn't covered by `cast_lossless`.
Might just be a bug. Want to file it?
>
> >> I'll leave it up to you what you want to do with this: add it to this
> >> series, make a new one, or let someone else handle it. If you don't want
> >> to handle it, let me know, then I'll create a good-first-issue :)
> >
> > I'll add a patch for `cast_lossless` -- the rest should probably go
> > into an issue.
>
> Do you mind filing the issue? Then you can decide yourself what you want
> to do yourself vs what you want to leave for others. Feel free to copy
> from my mail summary.
Well, I don't really know what's left to do. We're pretty close at
this point to having enabled everything but the nukes. Then there's
the strict provenance thing, which I suppose we can write down.
> Also I wouldn't mark it as a good-first-issue yet, since it's pretty
> complicated and needs to be delayed/based on this series.
Yeah, certainly not good-first-issue.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 0/6] rust: reduce pointer casts, enable related lints
2025-03-24 21:59 ` Tamir Duberstein
@ 2025-03-25 11:05 ` Benno Lossin
2025-03-25 11:10 ` Miguel Ojeda
2025-03-25 13:34 ` Tamir Duberstein
0 siblings, 2 replies; 55+ messages in thread
From: Benno Lossin @ 2025-03-25 11:05 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Mon Mar 24, 2025 at 10:59 PM CET, Tamir Duberstein wrote:
> On Mon, Mar 24, 2025 at 5:55 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> On Mon Mar 24, 2025 at 9:55 PM CET, Tamir Duberstein wrote:
>> > On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >> * `cast_lossless` (maybe this catches some of the `num as int_type`
>> >> conversions I mentioned above)
>> >
>> > Yeah, suggested the same above. I had hoped this would deal with the
>> > char as u32 pattern but it did not.
>>
>> Aw that's a shame. Maybe we should create a clippy issue for that,
>> thoughts?
>
> Yeah, it's not clear to me why it isn't covered by `cast_lossless`.
> Might just be a bug. Want to file it?
Done: https://github.com/rust-lang/rust-clippy/issues/14469
>> >> I'll leave it up to you what you want to do with this: add it to this
>> >> series, make a new one, or let someone else handle it. If you don't want
>> >> to handle it, let me know, then I'll create a good-first-issue :)
>> >
>> > I'll add a patch for `cast_lossless` -- the rest should probably go
>> > into an issue.
>>
>> Do you mind filing the issue? Then you can decide yourself what you want
>> to do yourself vs what you want to leave for others. Feel free to copy
>> from my mail summary.
>
> Well, I don't really know what's left to do. We're pretty close at
> this point to having enabled everything but the nukes. Then there's
> the strict provenance thing, which I suppose we can write down.
Yes, but there are also these from my original mail:
* `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
replace with `let ptr: *const ... = shared_ref;`. Don't know if there
is a clippy lint for this.
And the other points (haven't taken a look at the other series you
submitted, so I don't know to what extend you fixed the other `as` casts
I mentioned). So I figured you might know which ones we still have after
applying all your patches :)
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 0/6] rust: reduce pointer casts, enable related lints
2025-03-25 11:05 ` Benno Lossin
@ 2025-03-25 11:10 ` Miguel Ojeda
2025-03-25 13:34 ` Tamir Duberstein
1 sibling, 0 replies; 55+ messages in thread
From: Miguel Ojeda @ 2025-03-25 11:10 UTC (permalink / raw)
To: Benno Lossin
Cc: Tamir Duberstein, Masahiro Yamada, Nathan Chancellor,
Nicolas Schier, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
Björn Roy Baron, Andreas Hindborg, Alice Ryhl, Trevor Gross,
Danilo Krummrich, Greg Kroah-Hartman, Rafael J. Wysocki,
Brendan Higgins, David Gow, Rae Moar, Bjorn Helgaas,
Luis Chamberlain, Russ Weight, Rob Herring, Saravana Kannan,
linux-kbuild, linux-kernel, rust-for-linux, linux-kselftest,
kunit-dev, linux-pci, linux-block, devicetree
On Tue, Mar 25, 2025 at 12:05 PM Benno Lossin <benno.lossin@proton.me> wrote:
>
> Done: https://github.com/rust-lang/rust-clippy/issues/14469
Linked in our list:
https://github.com/Rust-for-Linux/linux/issues/349
Cheers,
Miguel
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 0/6] rust: reduce pointer casts, enable related lints
2025-03-25 11:05 ` Benno Lossin
2025-03-25 11:10 ` Miguel Ojeda
@ 2025-03-25 13:34 ` Tamir Duberstein
2025-03-25 15:33 ` Benno Lossin
1 sibling, 1 reply; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-25 13:34 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Tue, Mar 25, 2025 at 7:05 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Mon Mar 24, 2025 at 10:59 PM CET, Tamir Duberstein wrote:
> > On Mon, Mar 24, 2025 at 5:55 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >> On Mon Mar 24, 2025 at 9:55 PM CET, Tamir Duberstein wrote:
> >> > On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >> >> * `cast_lossless` (maybe this catches some of the `num as int_type`
> >> >> conversions I mentioned above)
> >> >
> >> > Yeah, suggested the same above. I had hoped this would deal with the
> >> > char as u32 pattern but it did not.
> >>
> >> Aw that's a shame. Maybe we should create a clippy issue for that,
> >> thoughts?
> >
> > Yeah, it's not clear to me why it isn't covered by `cast_lossless`.
> > Might just be a bug. Want to file it?
>
> Done: https://github.com/rust-lang/rust-clippy/issues/14469
Nice, looks like there's already a PR out:
https://github.com/rust-lang/rust-clippy/pull/14470.
> >> >> I'll leave it up to you what you want to do with this: add it to this
> >> >> series, make a new one, or let someone else handle it. If you don't want
> >> >> to handle it, let me know, then I'll create a good-first-issue :)
> >> >
> >> > I'll add a patch for `cast_lossless` -- the rest should probably go
> >> > into an issue.
> >>
> >> Do you mind filing the issue? Then you can decide yourself what you want
> >> to do yourself vs what you want to leave for others. Feel free to copy
> >> from my mail summary.
> >
> > Well, I don't really know what's left to do. We're pretty close at
> > this point to having enabled everything but the nukes. Then there's
> > the strict provenance thing, which I suppose we can write down.
>
> Yes, but there are also these from my original mail:
> * `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
> rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
> replace with `let ptr: *const ... = shared_ref;`. Don't know if there
> is a clippy lint for this.
I don't think we should go fixing things for which we don't have a
clippy lint. That way lies madness, particularly as patches begin to
be carried by other trees.
>
> And the other points (haven't taken a look at the other series you
> submitted, so I don't know to what extend you fixed the other `as` casts
> I mentioned). So I figured you might know which ones we still have after
> applying all your patches :)
>
> ---
> Cheers,
> Benno
>
^ permalink raw reply [flat|nested] 55+ messages in thread
* Re: [PATCH v5 0/6] rust: reduce pointer casts, enable related lints
2025-03-25 13:34 ` Tamir Duberstein
@ 2025-03-25 15:33 ` Benno Lossin
2025-03-25 17:17 ` Tamir Duberstein
0 siblings, 1 reply; 55+ messages in thread
From: Benno Lossin @ 2025-03-25 15:33 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Tue Mar 25, 2025 at 2:34 PM CET, Tamir Duberstein wrote:
> On Tue, Mar 25, 2025 at 7:05 AM Benno Lossin <benno.lossin@proton.me> wrote:
>> On Mon Mar 24, 2025 at 10:59 PM CET, Tamir Duberstein wrote:
>> > On Mon, Mar 24, 2025 at 5:55 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >> On Mon Mar 24, 2025 at 9:55 PM CET, Tamir Duberstein wrote:
>> >> > On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >> >> I'll leave it up to you what you want to do with this: add it to this
>> >> >> series, make a new one, or let someone else handle it. If you don't want
>> >> >> to handle it, let me know, then I'll create a good-first-issue :)
>> >> >
>> >> > I'll add a patch for `cast_lossless` -- the rest should probably go
>> >> > into an issue.
>> >>
>> >> Do you mind filing the issue? Then you can decide yourself what you want
>> >> to do yourself vs what you want to leave for others. Feel free to copy
>> >> from my mail summary.
>> >
>> > Well, I don't really know what's left to do. We're pretty close at
>> > this point to having enabled everything but the nukes. Then there's
>> > the strict provenance thing, which I suppose we can write down.
>>
>> Yes, but there are also these from my original mail:
>> * `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
>> rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
>> replace with `let ptr: *const ... = shared_ref;`. Don't know if there
>> is a clippy lint for this.
>
> I don't think we should go fixing things for which we don't have a
> clippy lint. That way lies madness, particularly as patches begin to
> be carried by other trees.
There already exists a lint for that: `clippy::ref_as_ptr` (almost
created an issue asking for one :)
Here is another lint that we probably want to enable (after the `&raw
{const,mut}` series lands): `clippy::borrow_as_ptr`.
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 0/6] rust: reduce pointer casts, enable related lints
2025-03-25 15:33 ` Benno Lossin
@ 2025-03-25 17:17 ` Tamir Duberstein
2025-03-25 20:22 ` Tamir Duberstein
2025-03-25 20:29 ` Benno Lossin
0 siblings, 2 replies; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-25 17:17 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Tue, Mar 25, 2025 at 11:33 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> On Tue Mar 25, 2025 at 2:34 PM CET, Tamir Duberstein wrote:
> > On Tue, Mar 25, 2025 at 7:05 AM Benno Lossin <benno.lossin@proton.me> wrote:
> >> On Mon Mar 24, 2025 at 10:59 PM CET, Tamir Duberstein wrote:
> >> > On Mon, Mar 24, 2025 at 5:55 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >> >> On Mon Mar 24, 2025 at 9:55 PM CET, Tamir Duberstein wrote:
> >> >> > On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
> >> >> >> I'll leave it up to you what you want to do with this: add it to this
> >> >> >> series, make a new one, or let someone else handle it. If you don't want
> >> >> >> to handle it, let me know, then I'll create a good-first-issue :)
> >> >> >
> >> >> > I'll add a patch for `cast_lossless` -- the rest should probably go
> >> >> > into an issue.
> >> >>
> >> >> Do you mind filing the issue? Then you can decide yourself what you want
> >> >> to do yourself vs what you want to leave for others. Feel free to copy
> >> >> from my mail summary.
> >> >
> >> > Well, I don't really know what's left to do. We're pretty close at
> >> > this point to having enabled everything but the nukes. Then there's
> >> > the strict provenance thing, which I suppose we can write down.
> >>
> >> Yes, but there are also these from my original mail:
> >> * `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
> >> rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
> >> replace with `let ptr: *const ... = shared_ref;`. Don't know if there
> >> is a clippy lint for this.
> >
> > I don't think we should go fixing things for which we don't have a
> > clippy lint. That way lies madness, particularly as patches begin to
> > be carried by other trees.
>
> There already exists a lint for that: `clippy::ref_as_ptr` (almost
> created an issue asking for one :)
🫡 picked this one up as well.
> Here is another lint that we probably want to enable (after the `&raw
> {const,mut}` series lands): `clippy::borrow_as_ptr`.
This sounds like a good one to file.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 0/6] rust: reduce pointer casts, enable related lints
2025-03-25 17:17 ` Tamir Duberstein
@ 2025-03-25 20:22 ` Tamir Duberstein
2025-03-25 20:29 ` Benno Lossin
1 sibling, 0 replies; 55+ messages in thread
From: Tamir Duberstein @ 2025-03-25 20:22 UTC (permalink / raw)
To: Benno Lossin
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Tue, Mar 25, 2025 at 1:17 PM Tamir Duberstein <tamird@gmail.com> wrote:
>
> On Tue, Mar 25, 2025 at 11:33 AM Benno Lossin <benno.lossin@proton.me> wrote:
>
> > Here is another lint that we probably want to enable (after the `&raw
> > {const,mut}` series lands): `clippy::borrow_as_ptr`.
>
> This sounds like a good one to file.
Actually I just enabled this and it has no incremental effect relative
to the lints already enabled in the series. We can enable it now, but
it seems to only trigger on patterns already caught by `ref_as_ptr`.
^ permalink raw reply [flat|nested] 55+ messages in thread* Re: [PATCH v5 0/6] rust: reduce pointer casts, enable related lints
2025-03-25 17:17 ` Tamir Duberstein
2025-03-25 20:22 ` Tamir Duberstein
@ 2025-03-25 20:29 ` Benno Lossin
1 sibling, 0 replies; 55+ messages in thread
From: Benno Lossin @ 2025-03-25 20:29 UTC (permalink / raw)
To: Tamir Duberstein
Cc: Masahiro Yamada, Nathan Chancellor, Nicolas Schier, Miguel Ojeda,
Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Danilo Krummrich,
Greg Kroah-Hartman, Rafael J. Wysocki, Brendan Higgins, David Gow,
Rae Moar, Bjorn Helgaas, Luis Chamberlain, Russ Weight,
Rob Herring, Saravana Kannan, linux-kbuild, linux-kernel,
rust-for-linux, linux-kselftest, kunit-dev, linux-pci,
linux-block, devicetree
On Tue Mar 25, 2025 at 6:17 PM CET, Tamir Duberstein wrote:
> On Tue, Mar 25, 2025 at 11:33 AM Benno Lossin <benno.lossin@proton.me> wrote:
>> On Tue Mar 25, 2025 at 2:34 PM CET, Tamir Duberstein wrote:
>> > On Tue, Mar 25, 2025 at 7:05 AM Benno Lossin <benno.lossin@proton.me> wrote:
>> >> On Mon Mar 24, 2025 at 10:59 PM CET, Tamir Duberstein wrote:
>> >> > On Mon, Mar 24, 2025 at 5:55 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >> >> On Mon Mar 24, 2025 at 9:55 PM CET, Tamir Duberstein wrote:
>> >> >> > On Mon, Mar 24, 2025 at 4:16 PM Benno Lossin <benno.lossin@proton.me> wrote:
>> >> >> >> I'll leave it up to you what you want to do with this: add it to this
>> >> >> >> series, make a new one, or let someone else handle it. If you don't want
>> >> >> >> to handle it, let me know, then I'll create a good-first-issue :)
>> >> >> >
>> >> >> > I'll add a patch for `cast_lossless` -- the rest should probably go
>> >> >> > into an issue.
>> >> >>
>> >> >> Do you mind filing the issue? Then you can decide yourself what you want
>> >> >> to do yourself vs what you want to leave for others. Feel free to copy
>> >> >> from my mail summary.
>> >> >
>> >> > Well, I don't really know what's left to do. We're pretty close at
>> >> > this point to having enabled everything but the nukes. Then there's
>> >> > the strict provenance thing, which I suppose we can write down.
>> >>
>> >> Yes, but there are also these from my original mail:
>> >> * `shared_ref as *const _` (for example in rust/kernel/uaccess.rs:247,
>> >> rust/kernel/str.rs:32 and rust/kernel/fs/file.rs:367), these we can
>> >> replace with `let ptr: *const ... = shared_ref;`. Don't know if there
>> >> is a clippy lint for this.
>> >
>> > I don't think we should go fixing things for which we don't have a
>> > clippy lint. That way lies madness, particularly as patches begin to
>> > be carried by other trees.
>>
>> There already exists a lint for that: `clippy::ref_as_ptr` (almost
>> created an issue asking for one :)
>
> 🫡 picked this one up as well.
Sniped yet again :)
Thanks a lot for adding this one as well.
>> Here is another lint that we probably want to enable (after the `&raw
>> {const,mut}` series lands): `clippy::borrow_as_ptr`.
>
> This sounds like a good one to file.
Since `raw_ref_op` is already enabled in rust-next, I just filed it:
https://github.com/Rust-for-Linux/linux/issues/1152
---
Cheers,
Benno
^ permalink raw reply [flat|nested] 55+ messages in thread