Re: [PATCH v5] tracing/eprobes: Allow use of BTF names to dereference pointers

[Google]

On Tue, 19 May 2026 14:17:26 -0400
Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:

> On Tue, 19 May 2026 17:48:47 +0000
> sashiko-bot@xxxxxxxxxx wrote:
> 
> > Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> > - [High] Using `(STRUCT)` typecasts for non-eprobes leaves the base fetch instruction uninitialized (`FETCH_OP_NOP`), silently breaking the probe.
> > - [High] Prematurely clearing `TPARG_FL_TYPECAST` causes cross-BTF type confusion and breaks string fetches.
> > - [Medium] Erroneous caching of `struct_btf` restricts a single trace probe to using typecasts from only one BTF object, preventing cross-module lookups.
> > - [Medium] Cross-BTF string table access in `parse_btf_arg()` causes invalid string comparisons.
> > - [Low] `clear_btf_context()` fails to clear `ctx->struct_btf` after dropping its reference.
> > --
> > 
> > commit de4b6287a7069fde24b0e0f679a545a06fb9ae6c
> > Author: Steven Rostedt <rostedt@xxxxxxxxxxx>
> > 
> > tracing/eprobes: Allow use of BTF names to dereference pointers
> > 
> > Add syntax to the parsing of eprobes to be able to typecast a trace event
> > field that is a pointer to a structure.
> > 
> > > diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
> > > index e1c73065dae51..2bc9f4ca8aa7e 100644
> > > --- a/kernel/trace/trace_probe.c
> > > +++ b/kernel/trace/trace_probe.c
> > > @@ -464,6 +470,27 @@ static const char *fetch_type_from_btf_type(struct btf *btf,
> > >  	return NULL;
> > >  }
> > >  
> > > +static int query_btf_struct(const char *sname, struct traceprobe_parse_context *ctx)
> > > +{
> > > +	int id;
> > > +
> > > +	if (!ctx->struct_btf) {
> > > +		struct btf *btf;
> > > +
> > > +		id = bpf_find_btf_id(sname, BTF_KIND_STRUCT, &btf);
> > > +		if (id < 0)
> > > +			return id;
> > > +		ctx->struct_btf = btf;
> > > +	} else {
> > > +		id = btf_find_by_name_kind(ctx->struct_btf, sname, BTF_KIND_STRUCT);  
> > 
> > [Severity: Medium]
> > Does caching struct_btf here restrict a single trace probe event to using
> > typecasts from only one BTF object?
> > 
> > If a probe attempts to typecast structures from two different kernel
> > modules, wouldn't the lookup for the second module's struct fail because
> > it only searches the cached BTF object of the first module (or vmlinux)?
> 
> Hmm, good point. I guess I should clear the struct_btf at the end of
> the '(' switch case.

Yes, it should be reset before parsing next argument.

> 
> > 
> > > +		if (id < 0)
> > > +			return id;
> > > +	}
> > > +
> > > +	ctx->last_struct = btf_type_by_id(ctx->struct_btf, id);
> > > +	return 0;
> > > +}  
> > 
> > [ ... ]
> > 
> > > @@ -515,6 +542,10 @@ static void clear_btf_context(struct traceprobe_parse_context *ctx)
> > >  		ctx->params = NULL;
> > >  		ctx->nr_params = 0;
> > >  	}
> > > +	if (ctx->struct_btf) {
> > > +		btf_put(ctx->struct_btf);
> > > +		ctx->last_struct = NULL;  
> > 
> > [Severity: Low]
> > Should ctx->struct_btf be explicitly set to NULL after btf_put() drops
> > the reference?
> 
> I'm thinking of dropping it in the '(' switch case.

Can you consider making the '(' switch case part as a helper
function because it depends on CONFIG_DEBUG_INFO_BTF?
 
Thanks,


-- 
Masami Hiramatsu (Google) <mhiramat@xxxxxxxxxx>