Re: [PATCH v4] tracing/probes: Allow use of BTF names to dereference pointers

[Steven Rostedt <rostedt@goodmis.org>]

On Tue, 19 May 2026 04:09:27 +0000
sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [High] NULL pointer dereference in `parse_btf_arg()` when typecasting `$retval`
> - [High] Instruction stream corruption when typecasting BTF variables with field dereferences
> - [High] Parser state corruption breaks multi-argument probes and standard BTF parameter resolution
> - [Medium] Hardcoded syntax check strictly prohibits typecasting hardware registers and standard BTF arguments
> - [Medium] Using `strrchr` causes parsing failures for memory dereferences containing parentheses
> --
> 
> commit 8ce6119a14206ed21706a773d9694dc24e53436e
> Author: Steven Rostedt <rostedt@goodmis.org>
> 
> tracing/probes: Allow use of BTF names to dereference pointers
> 
> This commit adds syntax to FETCHARG parsing to allow typecasting a value
> to a pointer to a structure. This allows users to rely on BTF to figure out
> member offsets instead of manually finding and hardcoding numerical offsets.
> 
> > But for event probes, or generic kprobes that records a register that
> > happens to be a pointer to a structure, they cannot dereference these
> > values with BTF naming, but must use numerical offsets.  
> 
> Does the code actually support dereferencing a register here? Hardware
> registers (like %rdi) and standard BTF arguments (like skb) do not begin
> with a $ character, but the parser explicitly enforces that the cast is
> followed by a $.

Heh, it doesn't support it yet. But I plan on doing so. I'll either add the
support or fix the change log.

> 
> > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > index e1c73065dae51..29b5a99074d76 100644
> > --- a/kernel/trace/trace_probe.c
> > +++ b/kernel/trace/trace_probe.c  
> [ ... ]
> > @@ -471,12 +491,12 @@ static int query_btf_context(struct traceprobe_parse_context *ctx)
> >  	struct btf *btf;
> >  	s32 nr;
> >  
> > -	if (ctx->btf)
> > -		return 0;
> > -
> >  	if (!ctx->funcname)
> >  		return -EINVAL;
> >  
> > +	if (ctx->btf)
> > +		return 0;
> > +  
> 
> If a user creates a probe with a struct cast for the return value (like
> (struct task_struct*)$retval), query_btf_struct() resolves the struct and
> sets ctx->btf but leaves ctx->proto as NULL.
> 
> When parse_btf_arg() later handles $retval, it calls query_btf_context(),
> which will return 0 here because ctx->btf is already set. Does this lead
> to parse_btf_arg() unconditionally dereferencing a NULL ctx->proto?
> 
> Additionally, for multi-argument probes where the first argument is a cast
> (like arg1=(struct foo*)$arg1 arg2=skb), setting ctx->btf for the first
> argument causes this check to return early for the second argument. Could
> this skip populating ctx->params and ctx->nr_params, causing valid BTF
> arguments to be rejected?

Yeah, this will be fixed by Masami's suggestion of using a different btf
for the structure.

> 
> [ ... ]
> > @@ -952,6 +986,12 @@ static int parse_probe_vars(char *orig_arg, const struct fetch_type *t,
> >  	int ret = 0;
> >  	int len;
> >  
> > +	if (ctx->flags & TPARG_FL_STRUCT) {
> > +		ret = parse_btf_arg(orig_arg, pcode, end, ctx);
> > +		if (ret < 0)
> > +			return ret;
> > +	}
> > +  
> 
> When parse_btf_arg() successfully parses a typecast with field dereferences,
> it emits a FETCH_OP_DEREF instruction and advances the *pcode pointer.
> 
> Since this block doesn't return upon success, execution continues and matches
> the base variable name later in parse_probe_vars(). The local code pointer
> is not updated to reflect the advanced *pcode, so it writes a new instruction
> (like FETCH_OP_ARG) into the same slot.
> 
> Will this overwrite the FETCH_OP_DEREF instruction and corrupt the trace probe
> bytecode?

Good catch. I need to move the code = *pcode; to after that if block.

> 
> [ ... ]
> > @@ -1231,6 +1271,43 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
> >  				code->op = FETCH_OP_IMM;
> >  		}
> >  		break;
> > +	case '(':
> > +		tmp = strrchr(arg, ')');  
> 
> If a user attempts to cast a standard memory dereference that itself
> contains parentheses (such as (struct foo*)+0($arg1)), strrchr() will
> find the final parenthesis of $arg1).

Ah, I should have used strchr(). Thanks.

> 
> Will this cause the pointer validation logic below to evaluate the wrong
> segment of the string and incorrectly fail with NO_PTR_STRCT?
> 
> > +		if (!tmp) {
> > +			trace_probe_log_err(ctx->offset + strlen(arg),
> > +					    DEREF_OPEN_BRACE);
> > +			return -EINVAL;
> > +		}
> > +
> > +		tmp--;
> > +		if (*tmp != '*') {
> > +			trace_probe_log_err(ctx->offset + (tmp - arg),
> > +					    NO_PTR_STRCT);
> > +			return -EINVAL;
> > +		}
> > +		*tmp = '\0';
> > +		ret = query_btf_struct(arg + 1, ctx);
> > +		*tmp = '*';
> > +
> > +		if (ret < 0) {
> > +			trace_probe_log_err(ctx->offset + 1, NO_PTR_STRCT);
> > +			return -EINVAL;
> > +		}
> > +
> > +		ctx->flags |= TPARG_FL_STRUCT;
> > +		tmp += 2;
> > +
> > +		if (*tmp != '$') {  
> 
> Is this strict check for '$' intentional?
> 
> As mentioned regarding the commit message, this prevents typecasting hardware
> registers and standard BTF arguments since they do not start with a '$',
> effectively restricting the feature to $retval, $stack, and $argN.

It was the only thing I tested. I'll remove it and test other options.

Thanks,

-- Steve

> 
> > +			trace_probe_log_err(ctx->offset + (tmp - arg),
> > +					    BAD_VAR);
> > +			return -EINVAL;
> > +		}
> > +
> > +		ctx->offset += tmp - arg;
> > +		ret = parse_probe_vars(tmp, type, pcode, end, ctx);
> > +		ctx->flags &= ~TPARG_FL_STRUCT;
> > +		ctx->last_struct = NULL;
> > +		break;  
>