Re: [PATCH v7 1/4] lib/vsprintf: Export functions for Rust pointer formatting support

[Alice Ryhl <aliceryhl@google.com>]

On Wed, Dec 31, 2025 at 10:46:37AM +0800, Ke Sun wrote:
> On Mon, Dec 29, 2025 at 6:44 PM Dirk Behme <dirk.behme@gmail.com> wrote:
> >
> > On 29.12.25 08:21, Ke Sun wrote:
> > > Export ptr_to_hashval() and extract kptr_restrict_value() to enable
> > > Rust code to use kernel pointer formatting functionality.
> > >
> > > - Export ptr_to_hashval() with EXPORT_SYMBOL_GPL for pointer hashing.
> > >   This function is used to hash kernel pointers before printing them,
> > >   preventing information leaks about kernel memory layout.
> > >
> > > - Extract kptr_restrict handling logic from restricted_pointer() into
> > >   a separate exported function kptr_restrict_value() that can be
> > >   reused by both restricted_pointer() (for %pK format specifier) and
> > >   Rust code.
> > >
> > > The kptr_restrict_value() function:
> > > - Returns 0, 1, or 2 to indicate the kptr_restrict sysctl value
> > > - Returns -1 for error case (IRQ context with kptr_restrict==1)
> > > - Modifies the pointer value through @pptr parameter when needed
> > >   (sets to NULL when access should be restricted)
> > >
> > > These functions are needed for the HashedPtr and RestrictedPtr wrapper
> > > types in rust/kernel/ptr.rs to provide safe pointer formatting that
> > > matches C kernel patterns.
> > >
> > > This refactoring:
> > > - Allows restricted_pointer() to reuse the common logic while
> > >   maintaining the same behavior
> > > - Enables Rust code to use the same kptr_restrict handling logic
> > > - Reduces code duplication
> > >
> > > Signed-off-by: Ke Sun <sunke@kylinos.cn>
> > > Suggested-by: Boqun Feng <boqun.feng@gmail.com>
> > > ---
> > >  include/linux/printk.h | 10 ++++++++
> > >  lib/vsprintf.c         | 55 ++++++++++++++++++++++++++++++++----------
> > >  2 files changed, 52 insertions(+), 13 deletions(-)
> > >
> > > diff --git a/include/linux/printk.h b/include/linux/printk.h
> > > index 45c663124c9bd..91380b1eff1a1 100644
> > > --- a/include/linux/printk.h
> > > +++ b/include/linux/printk.h
> > > @@ -385,6 +385,16 @@ extern void __printk_cpu_sync_put(void);
> > >
> > >  extern int kptr_restrict;
> > >
> > > +/**
> > > + * kptr_restrict_value - Determine what pointer value should be used for %pK
> > > + * @pptr: Pointer to the pointer value (may be modified)
> > > + *
> > > + * Returns:
> > > + *   - 0, 1, or 2: The kptr_restrict value if pointer should be used
> > > + *   - -1: Error case (IRQ context with kptr_restrict==1), *pptr unchanged
> > > + */
> > > +int kptr_restrict_value(const void **pptr);
> > > +
> > >  /**
> > >   * pr_fmt - used by the pr_*() macros to generate the printk format string
> > >   * @fmt: format string passed from a pr_*() macro
> > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> > > index a3790c43a0aba..29d2e681c6e73 100644
> > > --- a/lib/vsprintf.c
> > > +++ b/lib/vsprintf.c
> > > @@ -811,6 +811,7 @@ int ptr_to_hashval(const void *ptr, unsigned long *hashval_out)
> > >  {
> > >       return __ptr_to_hashval(ptr, hashval_out);
> > >  }
> > > +EXPORT_SYMBOL_GPL(ptr_to_hashval);
> > >
> > >  static char *ptr_to_id(char *buf, char *end, const void *ptr,
> > >                      struct printf_spec spec)
> > > @@ -857,14 +858,28 @@ static char *default_pointer(char *buf, char *end, const void *ptr,
> > >
> > >  int kptr_restrict __read_mostly;
> > >
> > > -static noinline_for_stack
> > > -char *restricted_pointer(char *buf, char *end, const void *ptr,
> > > -                      struct printf_spec spec)
> > > +/**
> > > + * kptr_restrict_value - Determine what pointer value should be used for %pK
> > > + * @pptr: Pointer to the pointer value (may be modified)
> > > + *
> > > + * This function determines what pointer value should be printed based on the
> > > + * kptr_restrict sysctl setting:
> > > + *
> > > + * - kptr_restrict == 0: Return 0 (use original pointer, will be hashed)
> > > + * - kptr_restrict == 1: Return 1 if current process has CAP_SYSLOG and same
> > > + *                       euid/egid, otherwise set *pptr to NULL and return 1
> > > + * - kptr_restrict >= 2: Set *pptr to NULL and return 2
> > > + *
> > > + * Returns:
> > > + *   - 0, 1, or 2: The kptr_restrict value if pointer should be used
> > > + *   - -1: Error case (IRQ context with kptr_restrict==1), *pptr unchanged
> > > + */
> > > +int kptr_restrict_value(const void **pptr)
> >
> > Would it be an improvment to not modify the pptr itself? But instead
> > to work with some kind of in ptr (not modified) and out_ptr (returned
> > to the caller then)?
> >
> > kptr_restrict_value(const void *ptr, const void **out_ptr)
> >
> > And an enum for the result? With something like
> >
> > enum kptr_restict_result {
> >   KPTR_RESTICT_ERROR = -1,
> >   KPTR_RESTICT_NONE = 0,
> >   KPTR_RESTICT_PARTIAL = 1,
> >   KPTR_RESTICT_FULL = 2,
> > }
> >
> > What would result in something like
> >
> > enum kptr_restict_result kptr_restrict_value(const void *ptr, const
> > void **out_ptr)
> >
> > Opinions?
> 
> Given that fully implementing all %p format specifier extensions would
> require substantial code changes, we're considering converting Rust's
> Formatter to C format strings at the Rust layer and directly calling
> snprintf. This would allow us to reuse the existing C implementations
> without modifying vsprintf.c.
> 
> What do you think of this approach?

That's never going to happen. It would be quite difficult to write a
safe API where you can't accidentally mix up the wrong type with the C
format string. You would probably have to write a proc macro parser for
format strings, which sounds like a nightmare. And you will never be
able to get error messages that are as good as the built-in formatting
machinery, because it relies on compiler internals and cannot be
reproduced in a custom proc macro.

I think Dirk's suggestion is reasonable. Or just do not support %pK.

Alice