Skip to contentSkip to conversation

tracing: Fixes for v7.1

PATCH New Steven Rostedt 1 participant 4 messages Apr 29

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover tracing: Fixes for v7.1

tracing fixes for v7.1:

- Fix inverted check of registering the stats for branch tracing

When calling register_stat_tracer() which returns zero on success and negative on error, the callers were checking the return of zero as an error and printing a warning message. Because this was just a normal printk() message and not a WARN(), it wasn't caught in any testing.

Fix the check to print the warning message when an error actually happens.

- Fix a typo in a comment in tracepoint.h

- Limit the size of event probes to 3K in size

It is possible to create a dynamic event probe via the tracefs system that is greater than the max size of an event that the ring buffer can hold. This basically causes the event to become useless. Limit the size of an event probe to be 3K as that should be large enough to handle any dynamic events being created, and fits within the PAGE_SIZE sub-buffers of the ring buffer.

git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
trace/fixes

Head SHA1: b2aa3b4d64e460ac606f386c24e7d8a873ce6f1a

Breno Leitao (1):
tracing: branch: Fix inverted check on stat tracer registration

Sheng Che Peng (1):
tracepoint: Fix typo in tracepoint.h comment

Steven Rostedt (1):
tracing/probes: Limit size of event probe to 3K

----

 include/linux/tracepoint.h  | 2 +-
 kernel/trace/trace_branch.c | 8 ++++----
 kernel/trace/trace_probe.c  | 6 ++++++
 kernel/trace/trace_probe.h  | 4 +++-
 4 files changed, 14 insertions(+), 6 deletions(-)
#1 lore raw

PATCH 1/3 tracing: branch: Fix inverted check on stat tracer registration

From: Breno Leitao <[email protected]>

init_annotated_branch_stats() and all_annotated_branch_stats() check the return value of register_stat_tracer() with "if (!ret)", but register_stat_tracer() returns 0 on success and a negative errno on failure. The inverted check causes the warning to be printed on every successful registration, e.g.:

Warning: could not register annotated branches stats

while leaving real failures silent. The initcall also returned a hard-coded 1 instead of the actual error.

Invert the check and propagate ret so that the warning fires on real errors and the initcall reports the correct status.

Cc Mathieu Desnoyers <[email protected]>
Cc Ingo Molnar <[email protected]>
Cc Frederic Weisbecker <[email protected]>
Fixes 002bb86d8d42 ("tracing/ftrace: separate events tracing and stats tracing engine")
---
 kernel/trace/trace_branch.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
kernel/trace/trace_branch.c +4 -4
index 6809b370e991..d1564db95a8f 100644
@@ -373,10 +373,10 @@ __init static int init_annotated_branch_stats(void)
 	int ret;
 
 	ret = register_stat_tracer(&annotated_branch_stats);
-	if (!ret) {
+	if (ret) {
 		printk(KERN_WARNING "Warning: could not register "
 				    "annotated branches stats\n");
-		return 1;
+		return ret;
 	}
 	return 0;
 }
@@ -438,10 +438,10 @@ __init static int all_annotated_branch_stats(void)
 	int ret;
 
 	ret = register_stat_tracer(&all_branch_stats);
-	if (!ret) {
+	if (ret) {
 		printk(KERN_WARNING "Warning: could not register "
 				    "all branches stats\n");
-		return 1;
+		return ret;
 	}
 	return 0;
 }
Signature · 1 line
2.53.0
#2 lore raw

PATCH 2/3 tracepoint: Fix typo in tracepoint.h comment

In reply to Steven Rostedt

From: Sheng Che Peng <[email protected]>

Change "my" to "may" in the description of subsystem configurations.

---
 include/linux/tracepoint.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
include/linux/tracepoint.h +1 -1
index 578e520b6ee6..763eea4d80d8 100644
@@ -202,7 +202,7 @@ static inline struct tracepoint *tracepoint_ptr_deref(tracepoint_ptr_t *p)
 #define TP_CONDITION(args...)	args
 
 /*
- * Individual subsystem my have a separate configuration to
+ * Individual subsystem may have a separate configuration to
  * enable their tracepoints. By default, this file will create
  * the tracepoints if CONFIG_TRACEPOINTS is defined. If a subsystem
  * wants to be able to disable its tracepoints from being created
Signature · 1 line
2.53.0
#3 lore raw

PATCH 3/3 tracing/probes: Limit size of event probe to 3K

In reply to Steven Rostedt

From: Steven Rostedt <[email protected]>

There currently isn't a max limit an event probe can be. One could make an event greater than PAGE_SIZE, which makes the event useless because if it's bigger than the max event that can be recorded into the ring buffer, then it will never be recorded.

A event probe should never need to be greater than 3K, so make that the max size. As long as the max is less than the max that can be recorded onto the ring buffer, it should be fine.

Cc Mathieu Desnoyers <[email protected]>
Fixes 93ccae7a22274 ("tracing/kprobes: Support basic types on dynamic events")
---
 kernel/trace/trace_probe.c | 6 ++++++
 kernel/trace/trace_probe.h | 4 +++-
 2 files changed, 9 insertions(+), 1 deletion(-)
kernel/trace/trace_probe.c +6 -0
index e1c73065dae5..e0d3a0da26af 100644
@@ -1523,6 +1523,12 @@ static int traceprobe_parse_probe_arg_body(const char *argv, ssize_t *size,
 	parg->offset = *size;
 	*size += parg->type->size * (parg->count ?: 1);
 
+	if (*size > MAX_PROBE_EVENT_SIZE) {
+		ret = -E2BIG;
+		trace_probe_log_err(ctx->offset, EVENT_TOO_BIG);
+		goto fail;
+	}
+
 	if (parg->count) {
 		len = strlen(parg->type->fmttype) + 6;
 		parg->fmt = kmalloc(len, GFP_KERNEL);
kernel/trace/trace_probe.h +3 -1
index 9fc56c937130..262d8707a3df 100644
@@ -38,6 +38,7 @@
 #define MAX_BTF_ARGS_LEN	128
 #define MAX_DENTRY_ARGS_LEN	256
 #define MAX_STRING_SIZE		PATH_MAX
+#define MAX_PROBE_EVENT_SIZE	3072
 
 /* Reserved field names */
 #define FIELD_STRING_IP		"__probe_ip"
@@ -561,7 +562,8 @@ extern int traceprobe_define_arg_fields(struct trace_event_call *event_call,
 	C(BAD_TYPE4STR,		"This type does not fit for string."),\
 	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(TOO_MANY_EARGS,	"Too many entry arguments specified"),	\
+	C(EVENT_TOO_BIG,	"Event too big (too many fields?)"),
 
 #undef C
 #define C(a, b)		TP_ERR_##a
Signature · 1 line
2.53.0