Skip to contentSkip to conversation

tracing/probes: Fix probe event argument parser

PATCH New Masami Hiramatsu (Google) 1 participant 3 messages Jan 23, 2024 Tracing

lore
Share & tools
lore

Masami Hiramatsu (Google)

author maintainer
#0 lore raw

cover tracing/probes: Fix probe event argument parser

These fix two bugs in the trace probe event argument parser, which I found while I'm cleaning up the code.

One is just adding an error message to message log buffer (but it is important to check the reason). Another is setting the size of argument entry correctly with argument data updated by BTF.

Thank you,

---

Masami Hiramatsu (Google) (2):
tracing/probes: Fix to show a parse error for bad type for $comm tracing/probes: Fix to set arg size and fmt after setting type from BTF

 kernel/trace/trace_probe.c |   32 ++++++++++++++++++--------------
 kernel/trace/trace_probe.h |    3 ++-
 2 files changed, 20 insertions(+), 15 deletions(-)

--
Masami Hiramatsu (Google) <[email protected]>

author maintainer
#1 lore raw

PATCH 1/2 tracing/probes: Fix to show a parse error for bad type for $comm

From: Masami Hiramatsu (Google) <[email protected]>

Fix to show a parse error for bad type (non-string) for $comm/$COMM and immediate-string. With this fix, error_log file shows appropriate error message as below.

/sys/kernel/tracing # echo 'p vfs_read $comm:u32' >> kprobe_events
sh: write error: Invalid argument
/sys/kernel/tracing # echo 'p vfs_read \"hoge":u32' >> kprobe_events
sh: write error: Invalid argument
/sys/kernel/tracing # cat error_log

[   30.144183] trace_kprobe: error: $comm and immediate-string only accepts string type
  Command: p vfs_read $comm:u32
                            ^
[   62.618500] trace_kprobe: error: $comm and immediate-string only accepts string type
  Command: p vfs_read \"hoge":u32
                              ^
Fixes 3dd1f7f24f8c ("tracing: probeevent: Fix to make the type of $comm string")
Cc [email protected]
Signed-off-by Masami Hiramatsu (Google) <[email protected]>
---
 kernel/trace/trace_probe.c |    7 +++++--
 kernel/trace/trace_probe.h |    3 ++-
 2 files changed, 7 insertions(+), 3 deletions(-)
kernel/trace/trace_probe.c +5 -2
index 4dc74d73fc1d..c6da5923e5b9 100644
@@ -1159,9 +1159,12 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
 	if (!(ctx->flags & TPARG_FL_TEVENT) &&
 	    (strcmp(arg, "$comm") == 0 || strcmp(arg, "$COMM") == 0 ||
 	     strncmp(arg, "\\\"", 2) == 0)) {
-		/* The type of $comm must be "string", and not an array. */
-		if (parg->count || (t && strcmp(t, "string")))
+		/* The type of $comm must be "string", and not an array type. */
+		if (parg->count || (t && strcmp(t, "string"))) {
+			trace_probe_log_err(ctx->offset + (t ? (t - arg) : 0),
+					NEED_STRING_TYPE);
 			goto out;
+		}
 		parg->type = find_fetch_type("string", ctx->flags);
 	} else
 		parg->type = find_fetch_type(t, ctx->flags);
kernel/trace/trace_probe.h +2 -1
index 850d9ecb6765..c1877d018269 100644
@@ -515,7 +515,8 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call,
 	C(BAD_HYPHEN,		"Failed to parse single hyphen. Forgot '>'?"),	\
 	C(NO_BTF_FIELD,		"This field is not found."),	\
 	C(BAD_BTF_TID,		"Failed to get BTF type info."),\
-	C(BAD_TYPE4STR,		"This type does not fit for string."),
+	C(BAD_TYPE4STR,		"This type does not fit for string."),\
+	C(NEED_STRING_TYPE,	"$comm and immediate-string only accepts string type"),
 
 #undef C
 #define C(a, b)		TP_ERR_##a
author maintainer
#2 lore raw

PATCH 2/2 tracing/probes: Fix to set arg size and fmt after setting type from BTF

In reply to Masami Hiramatsu (Google)

From: Masami Hiramatsu (Google) <[email protected]>

Since the BTF type setting updates probe_arg::type, the type size calculation and setting print-fmt should be done after that. Without this fix, the argument size and print-fmt can be wrong.

Fixes b576e09701c7 ("tracing/probes: Support function parameters if BTF is available")
Cc [email protected]
Signed-off-by Masami Hiramatsu (Google) <[email protected]>
---
 kernel/trace/trace_probe.c |   25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)
kernel/trace/trace_probe.c +13 -12
index c6da5923e5b9..34289f9c6707 100644
@@ -1172,18 +1172,6 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
 		trace_probe_log_err(ctx->offset + (t ? (t - arg) : 0), BAD_TYPE);
 		goto out;
 	}
-	parg->offset = *size;
-	*size += parg->type->size * (parg->count ?: 1);
-
-	ret = -ENOMEM;
-	if (parg->count) {
-		len = strlen(parg->type->fmttype) + 6;
-		parg->fmt = kmalloc(len, GFP_KERNEL);
-		if (!parg->fmt)
-			goto out;
-		snprintf(parg->fmt, len, "%s[%d]", parg->type->fmttype,
-			 parg->count);
-	}
 
 	code = tmp = kcalloc(FETCH_INSN_MAX, sizeof(*code), GFP_KERNEL);
 	if (!code)

@@ -1207,6 +1195,19 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
 				goto fail;
 		}
 	}
+	parg->offset = *size;
+	*size += parg->type->size * (parg->count ?: 1);
+
+	if (parg->count) {
+		len = strlen(parg->type->fmttype) + 6;
+		parg->fmt = kmalloc(len, GFP_KERNEL);
+		if (!parg->fmt) {
+			ret = -ENOMEM;
+			goto out;
+		}
+		snprintf(parg->fmt, len, "%s[%d]", parg->type->fmttype,
+			 parg->count);
+	}
 
 	ret = -EINVAL;
 	/* Store operation */