Rust for Linux List
 help / color / mirror / Atom feed
From: Dirk Behme <dirk.behme@gmail.com>
To: Ke Sun <sunke@kylinos.cn>, Miguel Ojeda <ojeda@kernel.org>,
	Petr Mladek <pmladek@suse.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	Timur Tabi <ttabi@nvidia.com>, Danilo Krummrich <dakr@kernel.org>,
	Benno Lossin <lossin@kernel.org>
Cc: "Boqun Feng" <boqun.feng@gmail.com>,
	"Gary Guo" <gary@garyguo.net>,
	"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
	"Andreas Hindborg" <a.hindborg@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"Trevor Gross" <tmgross@umich.edu>,
	"Tamir Duberstein" <tamird@gmail.com>,
	rust-for-linux@vger.kernel.org
Subject: Re: [PATCH v3 2/4] rust: kernel: Add pointer wrapper types for safe pointer formatting
Date: Thu, 25 Dec 2025 10:19:56 +0100	[thread overview]
Message-ID: <1e63cddd-7be2-4734-8c83-9a916a94a388@gmail.com> (raw)
In-Reply-To: <20251224081315.729684-3-sunke@kylinos.cn>

On 24.12.25 09:13, Ke Sun wrote:
> Add three pointer wrapper types (HashedPtr, RestrictedPtr, RawPtr) to
> rust/kernel/ptr.rs that correspond to C kernel's printk format specifiers
> %p, %pK, and %px. These types provide type-safe pointer formatting that
> matches C kernel patterns.
> 
> These wrapper types implement core::fmt::Pointer and delegate to the
> corresponding kernel formatting functions, enabling safe pointer
> formatting in Rust code that prevents information leaks about kernel
> memory layout.
> 
> Users can explicitly use these types:
>     pr_info!("{:p}\n", HashedPtr::from(ptr));
>     seq_print!(seq_file, "{:p}\n", RestrictedPtr::from(ptr));
>     pr_info!("{:p}\n", RawPtr::from(ptr));
> 
> Changes:
> - Add HashedPtr for %p (hashed, default behavior)
> - Add RestrictedPtr for %pK (restricted, respects kptr_restrict)
> - Add RawPtr for %px (raw address, debug only)
> - Add helper function kptr_restrict_value() in rust/helpers/fmt.c
> - Add impl_ptr_wrapper! macro to reduce code duplication
> 
> Signed-off-by: Ke Sun <sunke@kylinos.cn>
> ---
>  rust/helpers/fmt.c     |  61 ++++++++++++++
>  rust/helpers/helpers.c |   3 +-
>  rust/kernel/ptr.rs     | 186 +++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 249 insertions(+), 1 deletion(-)
>  create mode 100644 rust/helpers/fmt.c
....
> diff --git a/rust/kernel/ptr.rs b/rust/kernel/ptr.rs
> index e3893ed04049d..deb4d72bcecca 100644
> --- a/rust/kernel/ptr.rs
> +++ b/rust/kernel/ptr.rs
> @@ -1,11 +1,17 @@
>  // SPDX-License-Identifier: GPL-2.0
>  
>  //! Types and functions to work with pointers and addresses.
> +//!
> +//! This module provides wrapper types for formatting kernel pointers that correspond to the
> +//! C kernel's printk format specifiers `%p`, `%pK`, and `%px`.
>  
> +use core::fmt;
>  use core::mem::align_of;
>  use core::num::NonZero;
>  
> +use crate::bindings;
>  use crate::build_assert;
> +use crate::ffi::c_void;
>  
>  /// Type representing an alignment, which is always a power of two.
>  ///
> @@ -225,3 +231,183 @@ fn align_up(self, alignment: Alignment) -> Option<Self> {
>  }
>  
>  impl_alignable_uint!(u8, u16, u32, u64, usize);
> +
> +/// Placeholder string used when pointer hashing is not ready yet.
> +const PTR_PLACEHOLDER: &str = if core::mem::size_of::<*const c_void>() == 8 {
> +    "(____ptrval____)"
> +} else {
> +    "(ptrval)"
> +};
> +
> +/// Macro to implement common methods for pointer wrapper types.
> +macro_rules! impl_ptr_wrapper {
> +    ($($name:ident),* $(,)?) => {
> +        $(
> +            impl $name {
> +                /// Creates a new instance from a raw pointer.
> +                #[inline]
> +                pub fn from<T>(ptr: *const T) -> Self {
> +                    Self(ptr.cast())
> +                }
> +
> +                /// Creates a new instance from a mutable raw pointer.
> +                #[inline]
> +                pub fn from_mut<T>(ptr: *mut T) -> Self {
> +                    Self(ptr.cast())
> +                }
> +
> +                /// Returns the inner raw pointer.
> +                #[inline]
> +                pub fn as_ptr(&self) -> *const c_void {
> +                    self.0
> +                }
> +            }
> +
> +            impl<T> From<*const T> for $name {
> +                #[inline]
> +                fn from(ptr: *const T) -> Self {
> +                    Self::from(ptr)
> +                }
> +            }
> +
> +            impl<T> From<*mut T> for $name {
> +                #[inline]
> +                fn from(ptr: *mut T) -> Self {
> +                    Self::from_mut(ptr)
> +                }
> +            }
> +        )*
> +    };
> +}
> +
> +/// Helper function to hash a pointer and format it.
> +///
> +/// Returns `Ok(())` if the hash was successfully computed and formatted,
> +/// or the placeholder string if hashing is not ready yet.
> +fn format_hashed_ptr(ptr: *const c_void, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> +    // SAFETY: We're calling the kernel's ptr_to_hashval function which handles
> +    // hashing. This is safe as long as ptr is a valid pointer value.
> +    unsafe {

Would it be possible to limit the scope of the unsafe? E.g. to just

let ret = unsafe { bindings::ptr_to_hashval(ptr,
core::ptr::addr_of_mut!(hashval))};

> +        let mut hashval: crate::ffi::c_ulong = 0;
> +        let ret = bindings::ptr_to_hashval(ptr, core::ptr::addr_of_mut!(hashval));
> +
> +        if ret == 0 {
> +            // Successfully got hash value, format it
> +            write!(f, "{:p}", hashval as *const c_void)
> +        } else {
> +            // Hash not ready yet, print placeholder
> +            f.write_str(PTR_PLACEHOLDER)
> +        }

Whats about making the happy path the default and with this have less
indentation?

if ret != 0 {
     // Hash not ready yet, print placeholder
     return f.write_str(PTR_PLACEHOLDER);
}

// Successfully got hash value, format it
write!(f, "{:p}", hashval as *const c_void)


> +    }
> +}
> +
> +/// A pointer that will be hashed when printed (corresponds to `%p`).
> +///
> +/// This is the default behavior for kernel pointers - they are hashed to prevent
> +/// leaking information about the kernel memory layout.
> +///
> +/// # Example
> +///
> +/// ```
> +/// use kernel::ptr::HashedPtr;
> +///
> +/// let ptr = HashedPtr::from(0x12345678 as *const u8);
> +/// pr_info!("Pointer: {:p}\n", ptr);
> +/// ```
> +#[repr(transparent)]
> +#[derive(Copy, Clone, Debug)]
> +pub struct HashedPtr(*const c_void);
> +
> +impl fmt::Pointer for HashedPtr {
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> +        let ptr = self.0;
> +
> +        // Handle NULL pointers - print them directly
> +        if ptr.is_null() {
> +            return write!(f, "{:p}", ptr);
> +        }
> +
> +        format_hashed_ptr(ptr, f)
> +    }
> +}
> +
> +/// A pointer that will be restricted based on `kptr_restrict` when printed (corresponds to `%pK`).
> +///
> +/// This is intended for use in procfs/sysfs files that are read by userspace.
> +/// The behavior depends on the `kptr_restrict` sysctl setting.
> +///
> +/// # Example
> +///
> +/// ```
> +/// use kernel::ptr::RestrictedPtr;
> +///
> +/// let ptr = RestrictedPtr::from(0x12345678 as *const u8);
> +/// seq_print!(seq_file, "Pointer: {:p}\n", ptr);
> +/// ```

This example doesn't build:

error: cannot find macro `seq_print` in this scope
     --> rust/doctests_kernel_generated.rs:12492:1
      |
12492 | seq_print!(seq_file, "Pointer: {:p}\n", ptr);
      | ^^^^^^^^^
      |
help: consider importing this macro
      |
    3 + use kernel::seq_print;
      |

Adding that results in

error[E0425]: cannot find value `seq_file` in this scope
     --> rust/doctests_kernel_generated.rs:12493:12
      |
12493 | seq_print!(seq_file, "Pointer: {:p}\n", ptr);
      |            ^^^^^^^^ not found in this scope


> +#[repr(transparent)]
> +#[derive(Copy, Clone, Debug)]
> +pub struct RestrictedPtr(*const c_void);
> +
> +impl fmt::Pointer for RestrictedPtr {
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> +        let ptr = self.0;
> +
> +        // Handle NULL pointers
> +        if ptr.is_null() {
> +            return write!(f, "{:p}", ptr);
> +        }
> +
> +        // Use kptr_restrict_value to handle all kptr_restrict cases.
> +        // SAFETY: kptr_restrict_value handles capability checks and IRQ context.
> +        // - Returns NULL if no permission, IRQ context, or kptr_restrict >= 2
> +        // - Returns the original pointer if kptr_restrict == 0 (needs hashing)
> +        // - Returns the original pointer if kptr_restrict == 1 with permission (print raw)
> +        let restricted_ptr = unsafe { bindings::kptr_restrict_value(ptr) };
> +
> +        if restricted_ptr.is_null() {
> +            // No permission, IRQ context, or kptr_restrict >= 2 - print 0
> +            write!(f, "{:p}", core::ptr::null::<c_void>())
> +        } else {
> +            // restricted_ptr is non-null, meaning we should print something.
> +            // SAFETY: Reading kptr_restrict is safe as it's a kernel variable.
> +            let restrict = unsafe { bindings::kptr_restrict };
> +
> +            if restrict == 0 {
> +                // kptr_restrict == 0: hash the pointer (same as %p)
> +                format_hashed_ptr(ptr, f)
> +            } else {
> +                // kptr_restrict == 1 with permission: print the raw pointer directly (like %px)
> +                // This matches C behavior: pointer_string() prints the raw address
> +                write!(f, "{:p}", restricted_ptr)
> +            }
> +        }


Same if formatting like above here? E.g.

if restricted_ptr.is_null() {
    // No permission, IRQ context, or kptr_restrict >= 2 - print 0
    return write!(f, "{:p}", core::ptr::null::<c_void>());
}

// restricted_ptr is non-null, meaning we should print something.
// SAFETY: Reading kptr_restrict is safe as it's a kernel variable.
let restrict = unsafe { bindings::kptr_restrict };

if restrict == 0 {
    // kptr_restrict == 0: hash the pointer (same as %p)
    return format_hashed_ptr(ptr, f);
}

// kptr_restrict == 1 with permission: print the raw pointer directly
(like %px)
// This matches C behavior: pointer_string() prints the raw address
write!(f, "{:p}", restricted_ptr)


> +    }
> +}
> +
> +/// A pointer that will be printed as its raw address (corresponds to `%px`).
> +///
> +/// **Warning**: This exposes the real kernel address and should only be used
> +/// for debugging purposes. Consider using [`HashedPtr`] or [`RestrictedPtr`] instead.
> +///
> +/// # Example
> +///
> +/// ```
> +/// use kernel::ptr::RawPtr;
> +///
> +/// let ptr = RawPtr::from(0x12345678 as *const u8);
> +/// pr_info!("Debug pointer: {:p}\n", ptr);
> +/// ```
> +#[repr(transparent)]
> +#[derive(Copy, Clone, Debug)]
> +pub struct RawPtr(*const c_void);
> +
> +impl fmt::Pointer for RawPtr {
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
> +        // Directly format the raw address - no hashing or restriction
> +        // This corresponds to %px behavior
> +        write!(f, "{:p}", self.0)
> +    }
> +}
> +
> +// Implement common methods for all pointer wrapper types
> +impl_ptr_wrapper!(HashedPtr, RestrictedPtr, RawPtr);


Btw, would all this work with e.g. some format specifiers? E.g. with
{:016p} ?

Cheers,

Dirk


  reply	other threads:[~2025-12-25  9:20 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-24  8:13 [PATCH v3 0/4] rust: Add safe pointer formatting support Ke Sun
2025-12-24  8:13 ` [PATCH v3 1/4] lib/vsprintf: Export ptr_to_hashval for Rust use Ke Sun
2025-12-24  8:13 ` [PATCH v3 2/4] rust: kernel: Add pointer wrapper types for safe pointer formatting Ke Sun
2025-12-25  9:19   ` Dirk Behme [this message]
2025-12-25 15:22   ` kernel test robot
2025-12-24  8:13 ` [PATCH v3 3/4] rust: fmt: Default raw pointer formatting to HashedPtr Ke Sun
2025-12-24 20:58   ` kernel test robot
2025-12-25  2:50     ` Ke Sun
2025-12-24  8:13 ` [PATCH v3 4/4] docs: rust: Add pointer formatting documentation Ke Sun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1e63cddd-7be2-4734-8c83-9a916a94a388@gmail.com \
    --to=dirk.behme@gmail.com \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=boqun.feng@gmail.com \
    --cc=dakr@kernel.org \
    --cc=gary@garyguo.net \
    --cc=lossin@kernel.org \
    --cc=ojeda@kernel.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=sunke@kylinos.cn \
    --cc=tamird@gmail.com \
    --cc=tmgross@umich.edu \
    --cc=ttabi@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox