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

[Steven Rostedt <rostedt@goodmis.org>]

On Sun, 31 May 2026 10:14:58 +0900
Masami Hiramatsu (Google) <mhiramat@kernel.org> wrote:

> > > Does this prematurely release the BTF struct reference?
> > > If TPARG_FL_TYPECAST is unset here and ctx->struct_btf is put, won't
> > > later steps in traceprobe_parse_probe_arg_body() (like
> > > find_fetch_type_from_btf_type()) fail to properly infer struct field sizes?
> > > When ctx_btf(ctx) is called later without TPARG_FL_TYPECAST set, it
> > > will evaluate to ctx->btf (which is NULL for eprobes).
> > > Could this potentially lead to silent defaults, such as 64-bit reads for
> > > smaller fields, or fail to inject pointer dereferences for string fields,
> > > while also leaving ctx->last_type pointing to a prematurely released BTF
> > > object?  
> > 
> > Does this mean we need to set ctx->last_type to NULL here too?  
> 
> No, since the member we refer can be different from unsigned long.
> When we don't have ":type" suffix, we use BTF type information to
> decide appropriate type.
> 
> > 
> > Because everything above is pretty much the expected behavior. The put is
> > *not* premature. The last_struct and struct_btf are both set to NULL. I
> > guess the only thing missing is to reset last_type as well.  
> 
> No, as I explained, the last_type is used to determine the member type
> when user does not specify the ":type" suffix.
> 
> So, what we need to do is deferring the btf_put(struct_btf) as below:
> (no build test yet.)

OK, but I don't think we want the struct_btf to exist beyond a single
arg like the btf descriptor does. How about this (on top of this change),
where it clears the struct_btf at the end of traceprobe_parse_probe_arg_body()?

Also, I see the flag as being redundant and use the existence of
struct_btf to denote that it's parsing a typedef struct.

-- Steve

diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index 9246e9c3d066..56b7dc406ca1 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -397,8 +397,7 @@ static bool btf_type_is_char_array(struct btf *btf, const struct btf_type *type)
 
 static struct btf *ctx_btf(struct traceprobe_parse_context *ctx)
 {
-	return ctx->flags & TPARG_FL_TYPECAST ?
-		ctx->struct_btf : ctx->btf;
+	return ctx->struct_btf ? : ctx->btf;
 }
 
 static int check_prepare_btf_string_fetch(char *typename,
@@ -531,6 +530,15 @@ static int query_btf_context(struct traceprobe_parse_context *ctx)
 	return 0;
 }
 
+static void clear_struct_btf(struct traceprobe_parse_context *ctx)
+{
+	if (ctx->struct_btf) {
+		btf_put(ctx->struct_btf);
+		ctx->struct_btf = NULL;
+		ctx->last_struct = NULL;
+	}
+}
+
 static void clear_btf_context(struct traceprobe_parse_context *ctx)
 {
 	if (ctx->btf) {
@@ -579,7 +587,7 @@ static int parse_btf_field(char *fieldname, const struct btf_type *type,
 	struct fetch_insn *code = *pcode;
 	const struct btf_member *field;
 	u32 bitoffs, anon_offs;
-	bool is_struct = ctx->flags & TPARG_FL_TYPECAST;
+	bool is_struct = ctx->struct_btf != NULL;
 	struct btf *btf = ctx_btf(ctx);
 	char *next;
 	int is_ptr;
@@ -690,7 +698,7 @@ static int parse_btf_arg(char *varname,
 		ret = parse_trace_event(varname, code, ctx);
 		if (ret < 0)
 			return ret;
-		if (WARN_ON_ONCE(!(ctx->flags & TPARG_FL_TYPECAST)))
+		if (WARN_ON_ONCE(ctx->struct_btf == NULL))
 			return -EINVAL;
 		type = ctx->last_struct;
 		goto found_type;
@@ -804,21 +812,19 @@ static int parse_btf_bitfield(struct fetch_insn **pcode,
 
 static int query_btf_struct(const char *sname, struct traceprobe_parse_context *ctx)
 {
+	struct btf *btf = NULL;
 	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);
-		if (id < 0)
-			return id;
+	/* Could be a for a structure in a different module */
+	if (ctx->struct_btf) {
+		btf_put(ctx->struct_btf);
+		ctx->struct_btf = NULL;
 	}
 
+	id = bpf_find_btf_id(sname, BTF_KIND_STRUCT, &btf);
+	if (id < 0)
+		return id;
+	ctx->struct_btf = btf;
 	ctx->last_struct = btf_type_by_id(ctx->struct_btf, id);
 	return 0;
 }
@@ -848,25 +854,23 @@ static int handle_typecast(char *arg, struct fetch_insn **pcode,
 
 	if (ret < 0) {
 		trace_probe_log_err(ctx->offset + 1, NO_PTR_STRCT);
-		ret = -EINVAL;
-		goto out_put;
+		return -EINVAL;
 	}
 
-	ctx->flags |= TPARG_FL_TYPECAST;
 	tmp++;
 
 	ctx->offset += tmp - arg;
 	ret = parse_btf_arg(tmp, pcode, end, ctx);
-	ctx->flags &= ~TPARG_FL_TYPECAST;
-	ctx->last_struct = NULL;
-out_put:
-	btf_put(ctx->struct_btf);
-	ctx->struct_btf = NULL;
 	return ret;
 }
 
 #else /* !CONFIG_PROBE_EVENTS_BTF_ARGS */
 
+static void clear_struct_btf(struct traceprobe_parse_context *ctx)
+{
+	ctx->struct_btf = NULL;
+}
+
 static void clear_btf_context(struct traceprobe_parse_context *ctx)
 {
 	ctx->btf = NULL;
@@ -1673,6 +1677,9 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
 	}
 	kfree(tmp);
 
+	/* struct_btf should not be passed to other arguments */
+	clear_struct_btf(ctx);
+
 	return ret;
 }
 
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 952e3d7582b8..83565f1634db 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -394,7 +394,6 @@ static inline int traceprobe_get_entry_data_size(struct trace_probe *tp)
  * TPARG_FL_KERNEL and TPARG_FL_USER are also mutually exclusive.
  * TPARG_FL_FPROBE and TPARG_FL_TPOINT are optional but it should be with
  * TPARG_FL_KERNEL.
- * TPARG_FL_TYPECAST is set if an argument was typecast to a structure.
  */
 #define TPARG_FL_RETURN BIT(0)
 #define TPARG_FL_KERNEL BIT(1)
@@ -403,7 +402,6 @@ static inline int traceprobe_get_entry_data_size(struct trace_probe *tp)
 #define TPARG_FL_USER   BIT(4)
 #define TPARG_FL_FPROBE BIT(5)
 #define TPARG_FL_TPOINT BIT(6)
-#define TPARG_FL_TYPECAST BIT(7)
 #define TPARG_FL_LOC_MASK	GENMASK(4, 0)
 
 static inline bool tparg_is_function_entry(unsigned int flags)