On Sun, 31 May 2026 10:14:58 +0900
Masami Hiramatsu (Google) <[email protected]> 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)
