Linux Trace Kernel
 help / color / mirror / Atom feed
* [PATCH v6] tracing/eprobes: Allow use of BTF names to dereference pointers
@ 2026-05-22  2:50 Steven Rostedt
  2026-05-22 11:23 ` Steven Rostedt
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Steven Rostedt @ 2026-05-22  2:50 UTC (permalink / raw)
  To: LKML, Linux trace kernel
  Cc: Masami Hiramatsu, Mathieu Desnoyers, Mark Rutland, Peter Zijlstra,
	Namhyung Kim, Takaya Saeki, Douglas Raillard, Tom Zanussi,
	Andrew Morton, Thomas Gleixner, Ian Rogers, Jiri Olsa

From: Steven Rostedt <rostedt@goodmis.org>

Add syntax to the parsing of eprobes to be able to typecast a trace event
field that is a pointer to a structure.

Currently, a dereference must be a number, where the user has to figure
out manually the offset of a member of a structure that they want to
dereference.

But for event probes that records a field that happens to be a pointer to
a structure, it cannot dereference these values with BTF naming, but
must use numerical offsets.

For example, to find out what device a sk_buff is pointing to in the
net_dev_xmit trace event, one must first use gdb to find the offsets of the
members of the structures:

 (gdb) p &((struct sk_buff *)0)->dev
 $1 = (struct net_device **) 0x10
 (gdb) p &((struct net_device *)0)->name
 $2 = (char (*)[16]) 0x118

And then use the raw numbers to dereference:

  # echo 'e:xmit net.net_dev_xmit +0x118(+0x10($skbaddr)):string' >> dynamic_events

If BTF is in the kernel, then instead, the skbaddr can be typecast to
sk_buff and use the normal dereference logic.

  # echo 'e:xmit net.net_dev_xmit (sk_buff)skbaddr->dev->name:string' >> dynamic_events
  # echo 1 > events/eprobes/xmit/enable
  # cat trace
[..]
    sshd-session-1022    [000] b..2.   860.249343: xmit: (net.net_dev_xmit) arg1="enp7s0"
    sshd-session-1022    [000] b..2.   860.250061: xmit: (net.net_dev_xmit) arg1="enp7s0"
    sshd-session-1022    [000] b..2.   860.250142: xmit: (net.net_dev_xmit) arg1="enp7s0"
    sshd-session-1022    [000] b..2.   860.263553: xmit: (net.net_dev_xmit) arg1="enp7s0"
    sshd-session-1022    [000] b..2.   860.283820: xmit: (net.net_dev_xmit) arg1="enp7s0"
    sshd-session-1022    [000] b..2.   860.302716: xmit: (net.net_dev_xmit) arg1="enp7s0"
    sshd-session-1022    [000] b..2.   860.322905: xmit: (net.net_dev_xmit) arg1="enp7s0"
    sshd-session-1022    [000] b..2.   860.342828: xmit: (net.net_dev_xmit) arg1="enp7s0"
    sshd-session-1022    [000] b..2.   860.362268: xmit: (net.net_dev_xmit) arg1="enp7s0"
    sshd-session-1022    [000] b..2.   860.382335: xmit: (net.net_dev_xmit) arg1="enp7s0"
    sshd-session-1022    [000] b..2.   860.400856: xmit: (net.net_dev_xmit) arg1="enp7s0"
    sshd-session-1022    [000] b..2.   860.419893: xmit: (net.net_dev_xmit) arg1="enp7s0"

The syntax is simply: (STRUCT)(FIELD)->MEMBER[->MEMBER..]

Also add comments around the #else and #endif of #ifdef CONFIG_PROBE_EVENTS_BTF_ARGS
to know what they are for.

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---

Changes since v5: https://patch.msgid.link/20260519130144.40e71a00@fedora

- Restructure parse_btf_arg() to only use typecast when TEVENT flag is set
  (eprobes).

- Move found_type label to still test for (!type) in case ctx->last_struct
  is NULL for some reason.

- Restructure the code to be more specific to when to use ctx->struct_btf
  and use ctx->btf for places that the typecast will not be used.

- Move parse_trace_event() function outside of BTF #ifdef as it is used
  outside of it as well.

- Move the code in the '(' switch statement into a helper function called
  handle_typecast(). This will be a stub when BTF is not configured in.

- Have the ctx->last_struct and ctx->struct_btf only set within the
  handle_typecast() function, and have that function reset it too.

- Add comments to the #else and #endif of the #ifdef
  CONFIG_PROBE_EVENTS_BTF_ARGS to know what they are for.

 Documentation/trace/eprobetrace.rst |   4 +
 kernel/trace/trace_probe.c          | 171 +++++++++++++++++++++++-----
 kernel/trace/trace_probe.h          |   7 +-
 3 files changed, 152 insertions(+), 30 deletions(-)

diff --git a/Documentation/trace/eprobetrace.rst b/Documentation/trace/eprobetrace.rst
index 89b5157cfab8..fe3602540569 100644
--- a/Documentation/trace/eprobetrace.rst
+++ b/Documentation/trace/eprobetrace.rst
@@ -46,6 +46,10 @@ Synopsis of eprobe_events
 		  (x8/x16/x32/x64), VFS layer common type(%pd/%pD), "char",
                   "string", "ustring", "symbol", "symstr" and "bitfield" are
                   supported.
+  (STRUCT)FIELD->MEMBER[->MEMBER] : If BTF is supported, typecast FIELD to
+                  a pointer to STRUCT and then derference the pointer defined by
+                  ->MEMBER. Note that when this is used, the FIELD name does not
+                  need to be prefixed with a '$'.
 
 Types
 -----
diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c
index e0d3a0da26af..4a205cebda60 100644
--- a/kernel/trace/trace_probe.c
+++ b/kernel/trace/trace_probe.c
@@ -332,6 +332,25 @@ static int parse_trace_event_arg(char *arg, struct fetch_insn *code,
 	return -ENOENT;
 }
 
+static int parse_trace_event(char *arg, struct fetch_insn *code,
+			     struct traceprobe_parse_context *ctx)
+{
+	int ret;
+
+	if (code->data)
+		return -EFAULT;
+	ret = parse_trace_event_arg(arg, code, ctx);
+	if (!ret)
+		return 0;
+	if (strcmp(arg, "comm") == 0 || strcmp(arg, "COMM") == 0) {
+		code->op = FETCH_OP_COMM;
+		return 0;
+	}
+	/* backward compatibility */
+	ctx->offset = 0;
+	return -EINVAL;
+}
+
 #ifdef CONFIG_PROBE_EVENTS_BTF_ARGS
 
 static u32 btf_type_int(const struct btf_type *t)
@@ -376,11 +395,17 @@ static bool btf_type_is_char_array(struct btf *btf, const struct btf_type *type)
 		&& BTF_INT_BITS(intdata) == 8;
 }
 
+static struct btf *ctx_btf(struct traceprobe_parse_context *ctx)
+{
+	return ctx->flags & TPARG_FL_TYPECAST ?
+		ctx->struct_btf : ctx->btf;
+}
+
 static int check_prepare_btf_string_fetch(char *typename,
 				struct fetch_insn **pcode,
 				struct traceprobe_parse_context *ctx)
 {
-	struct btf *btf = ctx->btf;
+	struct btf *btf = ctx_btf(ctx);
 
 	if (!btf || !ctx->last_type)
 		return 0;
@@ -554,22 +579,29 @@ 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;
+	struct btf *btf = ctx_btf(ctx);
 	char *next;
 	int is_ptr;
 	s32 tid;
 
 	do {
-		/* Outer loop for solving arrow operator ('->') */
-		if (BTF_INFO_KIND(type->info) != BTF_KIND_PTR) {
-			trace_probe_log_err(ctx->offset, NO_PTR_STRCT);
-			return -EINVAL;
-		}
-		/* Convert a struct pointer type to a struct type */
-		type = btf_type_skip_modifiers(ctx->btf, type->type, &tid);
-		if (!type) {
-			trace_probe_log_err(ctx->offset, BAD_BTF_TID);
-			return -EINVAL;
+		if (!is_struct) {
+			/* Outer loop for solving arrow operator ('->') */
+			if (BTF_INFO_KIND(type->info) != BTF_KIND_PTR) {
+				trace_probe_log_err(ctx->offset, NO_PTR_STRCT);
+				return -EINVAL;
+			}
+
+			/* Convert a struct pointer type to a struct type */
+			type = btf_type_skip_modifiers(btf, type->type, &tid);
+			if (!type) {
+				trace_probe_log_err(ctx->offset, BAD_BTF_TID);
+				return -EINVAL;
+			}
 		}
+		/* Only the first type can skip being a pointer */
+		is_struct = false;
 
 		bitoffs = 0;
 		do {
@@ -580,7 +612,7 @@ static int parse_btf_field(char *fieldname, const struct btf_type *type,
 				return is_ptr;
 
 			anon_offs = 0;
-			field = btf_find_struct_member(ctx->btf, type, fieldname,
+			field = btf_find_struct_member(btf, type, fieldname,
 						       &anon_offs);
 			if (IS_ERR(field)) {
 				trace_probe_log_err(ctx->offset, BAD_BTF_TID);
@@ -602,7 +634,7 @@ static int parse_btf_field(char *fieldname, const struct btf_type *type,
 				ctx->last_bitsize = 0;
 			}
 
-			type = btf_type_skip_modifiers(ctx->btf, field->type, &tid);
+			type = btf_type_skip_modifiers(btf, field->type, &tid);
 			if (!type) {
 				trace_probe_log_err(ctx->offset, BAD_BTF_TID);
 				return -EINVAL;
@@ -627,6 +659,7 @@ static int parse_btf_field(char *fieldname, const struct btf_type *type,
 	return 0;
 }
 
+
 static int __store_entry_arg(struct trace_probe *tp, int argnum);
 
 static int parse_btf_arg(char *varname,
@@ -640,7 +673,7 @@ static int parse_btf_arg(char *varname,
 	int i, is_ptr, ret;
 	u32 tid;
 
-	if (WARN_ON_ONCE(!ctx->funcname))
+	if (WARN_ON_ONCE(!ctx->funcname && !(ctx->flags & TPARG_FL_TEVENT)))
 		return -EINVAL;
 
 	is_ptr = split_next_field(varname, &field, ctx);
@@ -653,6 +686,20 @@ static int parse_btf_arg(char *varname,
 		return -EOPNOTSUPP;
 	}
 
+	if (ctx->flags & TPARG_FL_TEVENT) {
+		int ret;
+
+		ret = parse_trace_event(varname, code, ctx);
+		if (ret < 0)
+			return ret;
+
+		if (ctx->flags & TPARG_FL_TYPECAST) {
+			type = ctx->last_struct;
+			goto found_type;
+		}
+		return 0;
+	}
+
 	if (ctx->flags & TPARG_FL_RETURN && !strcmp(varname, "$retval")) {
 		code->op = FETCH_OP_RETVAL;
 		/* Check whether the function return type is not void */
@@ -709,6 +756,7 @@ static int parse_btf_arg(char *varname,
 
 found:
 	type = btf_type_skip_modifiers(ctx->btf, tid, &tid);
+found_type:
 	if (!type) {
 		trace_probe_log_err(ctx->offset, BAD_BTF_TID);
 		return -EINVAL;
@@ -727,7 +775,7 @@ static int parse_btf_arg(char *varname,
 static const struct fetch_type *find_fetch_type_from_btf_type(
 					struct traceprobe_parse_context *ctx)
 {
-	struct btf *btf = ctx->btf;
+	struct btf *btf = ctx_btf(ctx);
 	const char *typestr = NULL;
 
 	if (btf && ctx->last_type)
@@ -758,7 +806,70 @@ static int parse_btf_bitfield(struct fetch_insn **pcode,
 	return 0;
 }
 
-#else
+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);
+		if (id < 0)
+			return id;
+	}
+
+	ctx->last_struct = btf_type_by_id(ctx->struct_btf, id);
+	return 0;
+}
+
+static int handle_typecast(char *arg, struct fetch_insn **pcode,
+			   struct fetch_insn *end,
+			   struct traceprobe_parse_context *ctx)
+{
+	char *tmp;
+	int ret;
+
+	/* Currently this only works for eprobes */
+	if (!(ctx->flags & TPARG_FL_TEVENT)) {
+		trace_probe_log_err(ctx->offset, TYPECAST_NOT_EVENT);
+		return -EINVAL;
+	}
+
+	tmp = strchr(arg, ')');
+	if (!tmp) {
+		trace_probe_log_err(ctx->offset + strlen(arg),
+				    DEREF_OPEN_BRACE);
+		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);
+		ret = -EINVAL;
+		goto out_put;
+	}
+
+	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);
+	return ret;
+}
+
+#else /* !CONFIG_PROBE_EVENTS_BTF_ARGS */
+
 static void clear_btf_context(struct traceprobe_parse_context *ctx)
 {
 	ctx->btf = NULL;
@@ -794,7 +905,15 @@ static int check_prepare_btf_string_fetch(char *typename,
 	return 0;
 }
 
-#endif
+static int handle_typecast(char *arg, struct fetch_insn **pcode,
+			   struct fetch_insn *end,
+			   struct traceprobe_parse_context *ctx)
+{
+	trace_probe_log_err(ctx->offset, NOSUP_BTFARG);
+	return -EOPNOTSUPP;
+}
+
+#endif /* CONFIG_PROBE_EVENTS_BTF_ARGS */
 
 #ifdef CONFIG_HAVE_FUNCTION_ARG_ACCESS_API
 
@@ -953,18 +1072,9 @@ static int parse_probe_vars(char *orig_arg, const struct fetch_type *t,
 	int len;
 
 	if (ctx->flags & TPARG_FL_TEVENT) {
-		if (code->data)
-			return -EFAULT;
-		ret = parse_trace_event_arg(arg, code, ctx);
-		if (!ret)
-			return 0;
-		if (strcmp(arg, "comm") == 0 || strcmp(arg, "COMM") == 0) {
-			code->op = FETCH_OP_COMM;
-			return 0;
-		}
-		/* backward compatibility */
-		ctx->offset = 0;
-		goto inval;
+		if (parse_trace_event(arg, code, ctx) < 0)
+			goto inval;
+		return 0;
 	}
 
 	if (str_has_prefix(arg, "retval")) {
@@ -1231,6 +1341,9 @@ parse_probe_arg(char *arg, const struct fetch_type *type,
 				code->op = FETCH_OP_IMM;
 		}
 		break;
+	case '(':
+		ret = handle_typecast(arg, pcode, end, ctx);
+		break;
 	default:
 		if (isalpha(arg[0]) || arg[0] == '_') {	/* BTF variable */
 			if (!tparg_is_function_entry(ctx->flags) &&
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 262d8707a3df..952e3d7582b8 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -394,6 +394,7 @@ 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)
@@ -402,6 +403,7 @@ 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)
@@ -422,7 +424,9 @@ struct traceprobe_parse_context {
 	const struct btf_param *params;	/* Parameter of the function */
 	s32 nr_params;			/* The number of the parameters */
 	struct btf *btf;		/* The BTF to be used */
+	struct btf *struct_btf;		/* The BTF to be used for structs */
 	const struct btf_type *last_type;	/* Saved type */
+	const struct btf_type *last_struct;	/* Saved structure */
 	u32 last_bitoffs;		/* Saved bitoffs */
 	u32 last_bitsize;		/* Saved bitsize */
 	struct trace_probe *tp;
@@ -563,7 +567,8 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call,
 	C(NEED_STRING_TYPE,	"$comm and immediate-string only accepts string type"),\
 	C(TOO_MANY_ARGS,	"Too many arguments are specified"),	\
 	C(TOO_MANY_EARGS,	"Too many entry arguments specified"),	\
-	C(EVENT_TOO_BIG,	"Event too big (too many fields?)"),
+	C(EVENT_TOO_BIG,	"Event too big (too many fields?)"),  \
+	C(TYPECAST_NOT_EVENT,	"Typecasts are only for eprobe fields"),
 
 #undef C
 #define C(a, b)		TP_ERR_##a
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v6] tracing/eprobes: Allow use of BTF names to dereference pointers
  2026-05-22  2:50 [PATCH v6] tracing/eprobes: Allow use of BTF names to dereference pointers Steven Rostedt
@ 2026-05-22 11:23 ` Steven Rostedt
  2026-05-22 14:45   ` Steven Rostedt
  2026-05-25 14:55 ` Masami Hiramatsu
  2026-05-26  0:09 ` Masami Hiramatsu
  2 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2026-05-22 11:23 UTC (permalink / raw)
  To: LKML, Linux trace kernel
  Cc: Masami Hiramatsu, Mathieu Desnoyers, Mark Rutland, Peter Zijlstra,
	Namhyung Kim, Takaya Saeki, Douglas Raillard, Tom Zanussi,
	Andrew Morton, Thomas Gleixner, Ian Rogers, Jiri Olsa,
	sashiko-bot@kernel.org, sashiko-reviews@lists.linux.dev


[ Replying to Sashiko: https://sashiko.dev/?list=org.kernel.vger.linux-trace-kernel#/patchset/20260521225033.56458336%40fedora ]

> > @@ -653,6 +686,20 @@ static int parse_btf_arg(char *varname,
> >  		return -EOPNOTSUPP;
> >  	}
> >  
> > +	if (ctx->flags & TPARG_FL_TEVENT) {
> > +		int ret;
> > +
> > +		ret = parse_trace_event(varname, code, ctx);
> > +		if (ret < 0)
> > +			return ret;

> When parse_trace_event() returns a negative error code (such as -EINVAL or
> -ENOENT) because a field name is invalid, the error is propagated back up
> the stack. Does this path miss calling trace_probe_log_err()? 
> If so, users might receive a generic failure without context or a caret
> pointing to the specific syntax error.

Hmm, there's a comment in the parse_trace_event() that sets ctx->offset for
backward compatibility. I'll investigate to see if we can fix that now.

> > +
> > +		if (ctx->flags & TPARG_FL_TYPECAST) {
> > +			type = ctx->last_struct;
> > +			goto found_type;
> > +		}
> > +		return 0;
> > +	}
> ...
> > @@ -758,7 +806,70 @@ static int parse_btf_bitfield(struct fetch_insn **pcode,
> >  	return 0;
> >  }
> >  
> > +static int handle_typecast(char *arg, struct fetch_insn **pcode,
> > +			   struct fetch_insn *end,
> > +			   struct traceprobe_parse_context *ctx)
> > +{
> ...
> > +	ctx->flags |= TPARG_FL_TYPECAST;
> > +	tmp++;
> > +
> > +	ctx->offset += tmp - arg;
> > +	ret = parse_btf_arg(tmp, pcode, end, ctx);
> > +	ctx->flags &= ~TPARG_FL_TYPECAST;

> Since TPARG_FL_TYPECAST is cleared here, will it break automatic type
> inference later? 
> The traceprobe_parse_context is used for subsequent steps like
> find_fetch_type_from_btf_type() and check_prepare_btf_string_fetch(),
> which execute in traceprobe_parse_probe_arg() after this function returns.
> These functions rely on ctx_btf(), which needs TPARG_FL_TYPECAST to be
> set to return ctx->struct_btf.

> > +	ctx->last_struct = NULL;
> > +out_put:
> > +	btf_put(ctx->struct_btf);

> Because ctx->struct_btf is not set to NULL after calling btf_put(),
> can this cause a refcount underflow and use-after-free?
> If a user creates an eprobe with multiple typecasted arguments, the shared
> traceprobe_parse_context means the second argument will enter
> query_btf_struct() and see ctx->struct_btf is not NULL. It will skip
> acquiring a new reference but still use the pointer. At the end of parsing
> the second argument, btf_put(ctx->struct_btf) will be called again
> unconditionally.

Oops, I forgot to do:

	ctx->struct_buf = NULL;

here.

Will fix.

Thanks,

-- Steve


> > +	return ret;
> > +}

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v6] tracing/eprobes: Allow use of BTF names to dereference pointers
  2026-05-22 11:23 ` Steven Rostedt
@ 2026-05-22 14:45   ` Steven Rostedt
  2026-05-24 10:15     ` Masami Hiramatsu
  0 siblings, 1 reply; 6+ messages in thread
From: Steven Rostedt @ 2026-05-22 14:45 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: LKML, Linux trace kernel, Mathieu Desnoyers, Mark Rutland,
	Peter Zijlstra, Namhyung Kim, Takaya Saeki, Douglas Raillard,
	Tom Zanussi, Andrew Morton, Thomas Gleixner, Ian Rogers,
	Jiri Olsa, sashiko-bot@kernel.org,
	sashiko-reviews@lists.linux.dev

On Fri, 22 May 2026 07:23:22 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> > > @@ -653,6 +686,20 @@ static int parse_btf_arg(char *varname,
> > >  		return -EOPNOTSUPP;
> > >  	}
> > >  
> > > +	if (ctx->flags & TPARG_FL_TEVENT) {
> > > +		int ret;
> > > +
> > > +		ret = parse_trace_event(varname, code, ctx);
> > > +		if (ret < 0)
> > > +			return ret;  
> 
> > When parse_trace_event() returns a negative error code (such as -EINVAL or
> > -ENOENT) because a field name is invalid, the error is propagated back up
> > the stack. Does this path miss calling trace_probe_log_err()? 
> > If so, users might receive a generic failure without context or a caret
> > pointing to the specific syntax error.  
> 
> Hmm, there's a comment in the parse_trace_event() that sets ctx->offset for
> backward compatibility. I'll investigate to see if we can fix that now.

Masami,

I looked at the code for parse_trace_event() that has:

	/* backward compatibility */
	ctx->offset = 0;
	return -EINVAL;

And it was originally introduced by commit 1b8b0cd754cd ("tracing/probes:
Move event parameter fetching code to common parser"), with:

+               ret = parse_trace_event_arg(arg, code, ctx);
+               if (!ret)
+                       return 0;
+               if (strcmp(arg, "comm") == 0 || strcmp(arg, "COMM") == 0) {
+                       code->op = FETCH_OP_COMM;
+                       return 0;
+               }
+               /* backward compatibility */
+               ctx->offset = 0;
+               goto inval;
+       }
+


What was the reason for the "backward compatibility"? Can we make it a real
error now?

-- Steve

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v6] tracing/eprobes: Allow use of BTF names to dereference pointers
  2026-05-22 14:45   ` Steven Rostedt
@ 2026-05-24 10:15     ` Masami Hiramatsu
  0 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2026-05-24 10:15 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: LKML, Linux trace kernel, Mathieu Desnoyers, Mark Rutland,
	Peter Zijlstra, Namhyung Kim, Takaya Saeki, Douglas Raillard,
	Tom Zanussi, Andrew Morton, Thomas Gleixner, Ian Rogers,
	Jiri Olsa, sashiko-bot@kernel.org,
	sashiko-reviews@lists.linux.dev

On Fri, 22 May 2026 10:45:21 -0400
Steven Rostedt <rostedt@kernel.org> wrote:

> On Fri, 22 May 2026 07:23:22 -0400
> Steven Rostedt <rostedt@goodmis.org> wrote:
> 
> > > > @@ -653,6 +686,20 @@ static int parse_btf_arg(char *varname,
> > > >  		return -EOPNOTSUPP;
> > > >  	}
> > > >  
> > > > +	if (ctx->flags & TPARG_FL_TEVENT) {
> > > > +		int ret;
> > > > +
> > > > +		ret = parse_trace_event(varname, code, ctx);
> > > > +		if (ret < 0)
> > > > +			return ret;  
> > 
> > > When parse_trace_event() returns a negative error code (such as -EINVAL or
> > > -ENOENT) because a field name is invalid, the error is propagated back up
> > > the stack. Does this path miss calling trace_probe_log_err()? 
> > > If so, users might receive a generic failure without context or a caret
> > > pointing to the specific syntax error.  
> > 
> > Hmm, there's a comment in the parse_trace_event() that sets ctx->offset for
> > backward compatibility. I'll investigate to see if we can fix that now.
> 
> Masami,
> 
> I looked at the code for parse_trace_event() that has:
> 
> 	/* backward compatibility */
> 	ctx->offset = 0;
> 	return -EINVAL;
> 
> And it was originally introduced by commit 1b8b0cd754cd ("tracing/probes:
> Move event parameter fetching code to common parser"), with:
> 
> +               ret = parse_trace_event_arg(arg, code, ctx);
> +               if (!ret)
> +                       return 0;
> +               if (strcmp(arg, "comm") == 0 || strcmp(arg, "COMM") == 0) {
> +                       code->op = FETCH_OP_COMM;
> +                       return 0;
> +               }
> +               /* backward compatibility */
> +               ctx->offset = 0;
> +               goto inval;
> +       }
> +
> 
> 
> What was the reason for the "backward compatibility"? Can we make it a real
> error now?

This is because a wrong eprobe syntax parser error position indicator.

In tools/testing/selftests/ftrace/test.d/dynevent/eprobes_syntax_errors.tc:

check_error 'e:foo/bar syscalls/sys_enter_openat arg=^dfd'      # BAD_FETCH_ARG
check_error 'e:foo/bar syscalls/sys_enter_openat ^arg=$foo'     # BAD_ATTACH_ARG

BAD_FETCH_ARG points the fetcharg name correctly, but the
BAD_ATTACH_ARG points wrong place in the test case.
I think we should fix test case. (Previously, since it was
a cleanup, I didn't changed it)

Thank you,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v6] tracing/eprobes: Allow use of BTF names to dereference pointers
  2026-05-22  2:50 [PATCH v6] tracing/eprobes: Allow use of BTF names to dereference pointers Steven Rostedt
  2026-05-22 11:23 ` Steven Rostedt
@ 2026-05-25 14:55 ` Masami Hiramatsu
  2026-05-26  0:09 ` Masami Hiramatsu
  2 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2026-05-25 14:55 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: LKML, Linux trace kernel, Masami Hiramatsu, Mathieu Desnoyers,
	Mark Rutland, Peter Zijlstra, Namhyung Kim, Takaya Saeki,
	Douglas Raillard, Tom Zanussi, Andrew Morton, Thomas Gleixner,
	Ian Rogers, Jiri Olsa

On Thu, 21 May 2026 22:50:33 -0400
Steven Rostedt <rostedt@kernel.org> wrote:

> +static int handle_typecast(char *arg, struct fetch_insn **pcode,
> +			   struct fetch_insn *end,
> +			   struct traceprobe_parse_context *ctx)
> +{
> +	char *tmp;
> +	int ret;
> +
> +	/* Currently this only works for eprobes */
> +	if (!(ctx->flags & TPARG_FL_TEVENT)) {
> +		trace_probe_log_err(ctx->offset, TYPECAST_NOT_EVENT);
> +		return -EINVAL;
> +	}
> +
> +	tmp = strchr(arg, ')');
> +	if (!tmp) {
> +		trace_probe_log_err(ctx->offset + strlen(arg),
> +				    DEREF_OPEN_BRACE);
> +		return -EINVAL;
> +	}
> +	*tmp = '\0';
> +	ret = query_btf_struct(arg + 1, ctx);
> +	*tmp = ')';

BTW, is there any reason to recover this? The @arg is copied
string, see traceprobe_parse_probe_arg_body().

Thanks,


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v6] tracing/eprobes: Allow use of BTF names to dereference pointers
  2026-05-22  2:50 [PATCH v6] tracing/eprobes: Allow use of BTF names to dereference pointers Steven Rostedt
  2026-05-22 11:23 ` Steven Rostedt
  2026-05-25 14:55 ` Masami Hiramatsu
@ 2026-05-26  0:09 ` Masami Hiramatsu
  2 siblings, 0 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2026-05-26  0:09 UTC (permalink / raw)
  To: Steven Rostedt
  Cc: LKML, Linux trace kernel, Masami Hiramatsu, Mathieu Desnoyers,
	Mark Rutland, Peter Zijlstra, Namhyung Kim, Takaya Saeki,
	Douglas Raillard, Tom Zanussi, Andrew Morton, Thomas Gleixner,
	Ian Rogers, Jiri Olsa

On Thu, 21 May 2026 22:50:33 -0400
Steven Rostedt <rostedt@kernel.org> wrote:

> @@ -640,7 +673,7 @@ static int parse_btf_arg(char *varname,
>  	int i, is_ptr, ret;
>  	u32 tid;
>  
> -	if (WARN_ON_ONCE(!ctx->funcname))
> +	if (WARN_ON_ONCE(!ctx->funcname && !(ctx->flags & TPARG_FL_TEVENT)))
>  		return -EINVAL;
>  
>  	is_ptr = split_next_field(varname, &field, ctx);
> @@ -653,6 +686,20 @@ static int parse_btf_arg(char *varname,
>  		return -EOPNOTSUPP;
>  	}
>  
> +	if (ctx->flags & TPARG_FL_TEVENT) {
> +		int ret;

nit: parse_btf_arg already declared @ret. So we don't need this.

Thanks,

-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-05-26  0:09 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-22  2:50 [PATCH v6] tracing/eprobes: Allow use of BTF names to dereference pointers Steven Rostedt
2026-05-22 11:23 ` Steven Rostedt
2026-05-22 14:45   ` Steven Rostedt
2026-05-24 10:15     ` Masami Hiramatsu
2026-05-25 14:55 ` Masami Hiramatsu
2026-05-26  0:09 ` Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox