public inbox for rust-for-linux@vger.kernel.org
 help / color / mirror / Atom feed
From: Boqun Feng <boqun.feng@gmail.com>
To: Ke Sun <sunke@kylinos.cn>
Cc: "Dirk Behme" <dirk.behme@gmail.com>,
	"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>, "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>,
	"Ke Sun" <sk.alvin.x@gmail.com>,
	rust-for-linux@vger.kernel.org
Subject: Re: [PATCH v6 2/4] rust: kernel: Add pointer wrapper types for safe pointer formatting
Date: Sat, 27 Dec 2025 13:56:11 +0800	[thread overview]
Message-ID: <aU90-0sVL81fJ2ON@tardis-2.local> (raw)
In-Reply-To: <20251227033958.3713232-3-sunke@kylinos.cn>

On Sat, Dec 27, 2025 at 11:39:53AM +0800, 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));
>     pr_info!("{:p}\n", RestrictedPtr::from(ptr));
>     pr_info!("{:p}\n", RawPtr::from(ptr));
> 
> Signed-off-by: Ke Sun <sunke@kylinos.cn>
> ---
>  rust/helpers/fmt.c     |  65 +++++++++++

I don't think the new function should be put in rust/helpers/, helpers
are for macros/inline functions that we cannot directly call via FFI,
they are basically mapping a Rust bindings call into a C function. So
the functions defined in rust/helpers/ should be simple, and
rust_helper_kptr_restrict_value() is not the case. Moreover, seems to me
that this function basically repeats part of the logic of
restricted_pointer(), hence it's not a good idea to duplicate this from
restricted_pointer().

I suggest you introduce a kptr_restrict_value() in lib/vsprintf.c and
make it EXPORT_GPL_SYMBOL, restricted_pointer() can reuse it as well,
you will probably need to rework kptr_restrict_value() to be something
like

	/*
	 * Return `kptr_restrict` (0, 1 or 2) if `*pptr` should be used
	 * for print. Otherwise return -1, which indicates the
	 * error_string() branch in restricted_pointer().
	 */
	int kptr_restrict_value(const void **pptr)

and restricted_pointer() can be:

	static noinline_for_stack
	char *restricted_pointer(char *buf, char *end, const void *ptr,
				 struct printf_spec spec)
	{
		int ret = kptr_restrict_value(&ptr);

		if (ret < 0) {
			if (spec.field_width == -1)
				spec.field_width = 2 * sizeof(ptr);
			return error_string(buf, end, "pK-error", spec);
		}

		if (ret == 0)
			return default_pointer(buf, end, ptr, spec);

		return pointer_string(buf, end, ptr, spec);
	}


With the new kptr_restrict_value() function, you can also avoid reading
`kptr_restrict` in `RestrictedPtr::fmt`, which is subject to data race
if I'm not missing anything.

Regards,
Boqun

>  rust/helpers/helpers.c |   3 +-
>  rust/kernel/ptr.rs     | 241 ++++++++++++++++++++++++++++++++++++++++-
>  3 files changed, 305 insertions(+), 4 deletions(-)
>  create mode 100644 rust/helpers/fmt.c
> 
> diff --git a/rust/helpers/fmt.c b/rust/helpers/fmt.c
> new file mode 100644
> index 0000000000000..4ee716bbfe284
> --- /dev/null
> +++ b/rust/helpers/fmt.c
> @@ -0,0 +1,65 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +/*
> + * Copyright (C) 2018 - 2025 KylinSoft Co., Ltd. All rights reserved.
> + * Copyright (C) 2018 - 2025 Ke Sun <sunke@kylinos.cn>
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/cred.h>
> +#include <linux/capability.h>
> +#include <linux/hardirq.h>
> +#include <linux/printk.h>
> +
> +/*
> + * Helper function for Rust to format a restricted pointer (%pK).
> + *
> + * This function determines what pointer value should be printed based on the
> + * kptr_restrict sysctl setting:
> + *
> + * - kptr_restrict == 0: Returns the original pointer (will be hashed by caller)
> + * - kptr_restrict == 1: Returns the original pointer if the current process has
> + *                       CAP_SYSLOG and same euid/egid, NULL otherwise
> + * - kptr_restrict >= 2: Always returns NULL
> + *
> + * Returns:
> + *   - The original pointer if it should be printed (case 0 or case 1 with permission)
> + *   - NULL if it should not be printed (no permission, IRQ context, or restrict >= 2)
> + */
> +const void *rust_helper_kptr_restrict_value(const void *ptr)
> +{
> +	switch (kptr_restrict) {
> +	case 0:
> +		/* Handle as %p - return original pointer for hashing */
> +		return ptr;
> +	case 1: {
> +		const struct cred *cred;
> +
> +		/*
> +		 * kptr_restrict==1 cannot be used in IRQ context because the
> +		 * capability check would be meaningless (no process context).
> +		 */
> +		if (in_hardirq() || in_serving_softirq() || in_nmi())
> +			return NULL;
> +
> +		/*
> +		 * Only return the real pointer value if the current process has
> +		 * CAP_SYSLOG and is running with the same credentials it started with.
> +		 * This prevents privilege escalation attacks where a process opens a
> +		 * file with %pK, then elevates privileges before reading it.
> +		 */
> +		cred = current_cred();
> +		if (!has_capability_noaudit(current, CAP_SYSLOG) ||
> +		    !uid_eq(cred->euid, cred->uid) ||
> +		    !gid_eq(cred->egid, cred->gid))
> +			return NULL;
> +		break;
> +	}
> +	case 2:
> +	default:
> +		/* Always hide pointer values when kptr_restrict >= 2 */
> +		return NULL;
> +	}
> +
> +	return ptr;
> +}
> diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c
> index 79c72762ad9c4..b6877fe8dafc0 100644
> --- a/rust/helpers/helpers.c
> +++ b/rust/helpers/helpers.c
> @@ -27,8 +27,9 @@
>  #include "dma.c"
>  #include "drm.c"
>  #include "err.c"
> -#include "irq.c"
> +#include "fmt.c"
>  #include "fs.c"
> +#include "irq.c"
>  #include "io.c"
>  #include "jump_label.c"
>  #include "kunit.c"
> diff --git a/rust/kernel/ptr.rs b/rust/kernel/ptr.rs
> index e3893ed04049d..8b58221c2ec4b 100644
> --- a/rust/kernel/ptr.rs
> +++ b/rust/kernel/ptr.rs
> @@ -1,11 +1,22 @@
>  // 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::mem::align_of;
> -use core::num::NonZero;
> +use core::{
> +    fmt,
> +    fmt::Pointer,
> +    mem::align_of,
> +    num::NonZero, //
> +};
>  
> -use crate::build_assert;
> +use crate::{
> +    bindings,
> +    build_assert,
> +    ffi::c_void, //
> +};
>  
>  /// Type representing an alignment, which is always a power of two.
>  ///
> @@ -225,3 +236,227 @@ 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 {
> +    let mut hashval: crate::ffi::c_ulong = 0;
> +    // 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.
> +    let ret = unsafe { bindings::ptr_to_hashval(ptr, core::ptr::addr_of_mut!(hashval)) };
> +
> +    if ret != 0 {
> +        // Hash not ready yet, print placeholder with formatting options applied
> +        // Using `pad()` ensures width, alignment, and padding options are respected
> +        return f.pad(PTR_PLACEHOLDER);
> +    }
> +
> +    // Successfully got hash value, format it using Pointer::fmt to preserve
> +    // formatting options (width, alignment, padding, etc.)
> +    Pointer::fmt(&(hashval as *const c_void), f)
> +}
> +
> +/// 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::{
> +///     prelude::fmt,
> +///     ptr::HashedPtr,
> +///     str::CString, //
> +/// };
> +///
> +/// let ptr = HashedPtr::from(0x12345678 as *const u8);
> +/// pr_info!("Hashed pointer: {:016p}\n", ptr);
> +///
> +/// // Width option test
> +/// let cstr = CString::try_from_fmt(fmt!("{:30p}", ptr))?;
> +/// let width_30 = cstr.to_str()?;
> +/// assert_eq!(width_30.len(), 30);
> +/// # Ok::<(), kernel::error::Error>(())
> +/// ```
> +#[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 {
> +        // Handle NULL pointers - print them directly
> +        if self.0.is_null() {
> +            return Pointer::fmt(&self.0, f);
> +        }
> +
> +        format_hashed_ptr(self.0, 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::{
> +///     prelude::fmt,
> +///     ptr::RestrictedPtr,
> +///     str::CString, //
> +/// };
> +///
> +/// let ptr = RestrictedPtr::from(0x12345678 as *const u8);
> +/// pr_info!("Restricted pointer: {:016p}\n", ptr);
> +///
> +/// // Width option test
> +/// let cstr = CString::try_from_fmt(fmt!("{:30p}", ptr))?;
> +/// let width_30 = cstr.to_str()?;
> +/// assert_eq!(width_30.len(), 30);
> +/// # Ok::<(), kernel::error::Error>(())
> +/// ```
> +#[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 {
> +        // Handle NULL pointers
> +        if self.0.is_null() {
> +            return Pointer::fmt(&self.0, f);
> +        }
> +
> +        // 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(self.0) };
> +
> +        if restricted_ptr.is_null() {
> +            // No permission, IRQ context, or kptr_restrict >= 2 - print 0
> +            return Pointer::fmt(&core::ptr::null::<c_void>(), f);
> +        }
> +
> +        // 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(self.0, f)
> +        } else {
> +            // kptr_restrict == 1 with permission: print the raw pointer directly (like %px)
> +            // This matches C behavior: pointer_string() prints the raw address
> +            Pointer::fmt(&restricted_ptr, f)
> +        }
> +    }
> +}
> +
> +/// 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::{
> +///     prelude::fmt,
> +///     ptr::RawPtr,
> +///     str::CString, //
> +/// };
> +///
> +/// let ptr = RawPtr::from(0x12345678 as *const u8);
> +///
> +/// // Basic formatting
> +/// let cstr = CString::try_from_fmt(fmt!("{:p}", ptr))?;
> +/// let formatted = cstr.to_str()?;
> +/// assert_eq!(formatted, "0x12345678");
> +///
> +/// // Right align with zero padding, width 30
> +/// let cstr = CString::try_from_fmt(fmt!("{:0>30p}", ptr))?;
> +/// let right_zero = cstr.to_str()?;
> +/// assert_eq!(right_zero, "000000000000000000000x12345678");
> +///
> +/// // Left align with zero padding, width 30
> +/// let cstr = CString::try_from_fmt(fmt!("{:0<30p}", ptr))?;
> +/// let left_zero = cstr.to_str()?;
> +/// assert_eq!(left_zero, "0x1234567800000000000000000000");
> +///
> +/// // Center align with zero padding, width 30
> +/// let cstr = CString::try_from_fmt(fmt!("{:0^30p}", ptr))?;
> +/// let center_zero = cstr.to_str()?;
> +/// assert_eq!(center_zero, "00000000000x123456780000000000");
> +/// # Ok::<(), kernel::error::Error>(())
> +/// ```
> +#[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.
> +        Pointer::fmt(&self.0, f)
> +    }
> +}
> +
> +// Implement common methods for all pointer wrapper types
> +impl_ptr_wrapper!(
> +    HashedPtr,
> +    RawPtr,
> +    RestrictedPtr, //
> +);
> -- 
> 2.43.0
> 

  reply	other threads:[~2025-12-27  5:56 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-12-27  3:39 [PATCH v6 0/4] rust: Add safe pointer formatting support Ke Sun
2025-12-27  3:39 ` [PATCH v6 1/4] lib/vsprintf: Export ptr_to_hashval for Rust use Ke Sun
2025-12-27  3:39 ` [PATCH v6 2/4] rust: kernel: Add pointer wrapper types for safe pointer formatting Ke Sun
2025-12-27  5:56   ` Boqun Feng [this message]
2025-12-29  7:38     ` Ke Sun
2025-12-29  6:44   ` Dirk Behme
2025-12-29  7:34     ` Ke Sun
2025-12-27  3:39 ` [PATCH v6 3/4] rust: fmt: Default raw pointer formatting to HashedPtr Ke Sun
2025-12-27  3:39 ` [PATCH v6 4/4] docs: rust: Add pointer formatting documentation Ke Sun
2025-12-27  9:12 ` [PATCH v6 0/4] rust: Add safe pointer formatting support Dirk Behme

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=aU90-0sVL81fJ2ON@tardis-2.local \
    --to=boqun.feng@gmail.com \
    --cc=a.hindborg@kernel.org \
    --cc=aliceryhl@google.com \
    --cc=bjorn3_gh@protonmail.com \
    --cc=dakr@kernel.org \
    --cc=dirk.behme@gmail.com \
    --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=sk.alvin.x@gmail.com \
    --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