Re: [PATCH perf-tools-next v3 2/4] perf trace: Auto-assign kernel symbol beautifier to function pointer fields

On Thu, Aug 20, 2026 at 09:26:12PM +0000, [email protected] wrote:
> commit d3017d506b83550526e2692582e2544d0b006d0f
> Author: Aaron Tomlin <[email protected]>
> 
> perf trace: Auto-assign kernel symbol beautifier to function pointer fields
> 
> This patch enhances perf trace to automatically format kernel function
> pointer fields using the SCA_KSYM beautifier. It detects these fields
> by type signature or by matching common function pointer and callback
> field names in a lookup table.
> 
> > diff --git a/tools/perf/builtin-trace.c b/tools/perf/builtin-trace.c
> > index 0030489465032..bb8f4fd9ba244 100644
> > --- a/tools/perf/builtin-trace.c
> > +++ b/tools/perf/builtin-trace.c
> > @@ -2096,6 +2096,17 @@ static int syscall__alloc_arg_fmts(struct syscall *sc, int nr_args)
> >  }
> >  
> >  static const struct syscall_arg_fmt syscall_arg_fmts__by_name[] = {
> > +	{ .name = "action",	.scnprintf = SCA_KSYM, },
> > +	{ .name = "call_site",	.scnprintf = SCA_KSYM, },
> > +	{ .name = "callback",	.scnprintf = SCA_KSYM, },
> > +	{ .name = "caller",	.scnprintf = SCA_KSYM, },
> > +	{ .name = "callsite",	.scnprintf = SCA_KSYM, },
> > +	{ .name = "cb",		.scnprintf = SCA_KSYM, },
> 
> [ ... ]
> 
> > @@ -2230,6 +2220,24 @@ syscall_arg_fmt__init_array(struct syscall_arg_fmt *arg, struct tep_format_field
> >  			if (fmt) {
> >  				arg->scnprintf = fmt->scnprintf;
> >  				arg->strtoul   = fmt->strtoul;
> > +			} else if ((field->flags & TEP_FIELD_IS_POINTER) || strstr(field->name, "addr") ||
> > +				   field_has_hex_fmt(field, len)) {
> > +				arg->scnprintf = SCA_PTR;
> > +			} else if (strcmp(field->type, "pid_t") == 0) {
> > +				arg->scnprintf = SCA_PID;
> > +			} else if (strcmp(field->type, "umode_t") == 0) {
> > +				arg->scnprintf = SCA_MODE_T;
> > +			} else if ((field->flags & TEP_FIELD_IS_ARRAY) && strstr(field->type, "char")) {
> > +				arg->scnprintf = SCA_CHAR_ARRAY;
> > +				arg->nr_entries = field->arraylen;
> 
> [Severity: High]
> Does checking syscall_arg_fmt__find_by_name() earlier in this branch,
> before the type-based checks like string arrays or BTF enums, short-circuit
> those checks for generic field names?
> 
> If a field is named "caller", "action", or "cb" but is actually a dynamic
> string or integer, it seems this early name match will blindly assign
> SCA_KSYM.
> 
> Would this cause strings to be printed as raw kernel addresses by passing a
> userspace perf pointer to SCA_KSYM, permanently obfuscating the trace data?

I agree. This is a valid concern.

To resolve, I will guard the SCA_KSYM assignment in
syscall_arg_fmt__init_array(). Moving forward, a name-based lookup now
assigns SCA_KSYM only if the field is an actual pointer
(TEP_FIELD_IS_POINTER) or a pointer-sized non-array scalar (field->size ==
sizeof(void *) && !(field->flags & TEP_FIELD_IS_ARRAY)). If a field fails
this guard, it falls through to subsequent type checks (i.e.,
SCA_CHAR_ARRAY, BTF enum inspection, SCA_FD, etc.). Non-SCA_KSYM entries in
the lookup table (e.g., "msr" and "vector") continue to pass through
unhindered. This should work.


> [ ... ]
> 
> > @@ -3297,12 +3305,6 @@ static unsigned char bitmap_byte(const unsigned long *mask, int byte_idx)
> >  	return b_val;
> >  }
> >  
> > -static bool trace__field_is_ip(const char *name)
> > -{
> > -	return !strcmp(name, "__probe_ip") ||
> > -	       !strcmp(name, "caller_ip") ||
> > -	       !strcmp(name, "call_site");
> > -}
> 
> [Severity: Medium]
> Since trace__field_is_ip() is removed here, should "caller_ip" have been
> added to the new syscall_arg_fmts__by_name lookup table?
> 
> Because "caller_ip" is typed as unsigned long (not a pointer), lacks "addr"
> in its name, and its TP_printk format strings do not match the strict
> =0x/=%p checks in field_has_hex_fmt(), it evades all heuristics.
> 
> Will this cause kernel instruction pointers like "caller_ip" (used heavily
> in XFS and OCFS2 tracepoints) to be formatted as unreadable base-10 decimal
> integers?

Apologies, "caller_ip" was accidently omitted!

In the next iteration, I have added "caller_ip" to
syscall_arg_fmts__by_name[] in strict alphabetical order to ensure
bsearch() integrity. Furthermore, I have relaxed field_has_hex_fmt() to
tolerate delimiters preceding %p/%# format specifiers, ensuring tracepoint
formats utilising %pS/%ps are properly recognised too.


Kind regards,
-- 
Aaron Tomlin