Skip to contentSkip to conversation

tracing/user_events: Fix struct arg size match check

PATCH New Beau Belgrave 1 participant 3 messages Jun 29, 2023

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

cover tracing/user_events: Fix struct arg size match check

The edge case of struct fields having an extra size argument got missed when checking if events have the same name and arguments. This lead to events not being able to register if a matching event name was already created with a struct argument.

This series simply fixes this and adds a self test to ensure we do not miss it again.

Beau Belgrave (2):
tracing/user_events: Fix struct arg size match check selftests/user_events: Test struct size match cases

 kernel/trace/trace_events_user.c               |  3 +++
 tools/testing/selftests/user_events/dyn_test.c | 12 ++++++++++++
 2 files changed, 15 insertions(+)

base-commit: 6995e2de6891c724bfeb2db33d7b87775f913ad1

Signature · 1 line
2.34.1
#1 lore raw

PATCH 1/2 tracing/user_events: Fix struct arg size match check

When users register an event the name of the event and it's argument are checked to ensure they match if the event already exists. Normally all arguments are in the form of "type name", except for when the type starts with "struct ". In those cases, the size of the struct is passed in addition to the name, IE: "struct my_struct a 20" for an argument that is of type "struct my_struct" with a field name of "a" and has the size of 20 bytes.

The current code does not honor the above case properly when comparing a match. This causes the event register to fail even when the same string was used for events that contain a struct argument within them. The example above "struct my_struct a 20" generates a match string of "struct my_struct a" omitting the size field.

Add the struct size of the existing field when generating a comparison string for a struct field to ensure proper match checking.

Fixes e6f89a149872 ("tracing/user_events: Ensure user provided strings are safely formatted")
---
 kernel/trace/trace_events_user.c | 3 +++
 1 file changed, 3 insertions(+)
kernel/trace/trace_events_user.c +3 -0
index 8df0550415e7..5c787fe48d53 100644
@@ -1317,6 +1317,9 @@ static int user_field_set_string(struct ftrace_event_field *field,
 	pos += snprintf(buf + pos, LEN_OR_ZERO, " ");
 	pos += snprintf(buf + pos, LEN_OR_ZERO, "%s", field->name);
 
+	if (str_has_prefix(field->type, "struct "))
+		pos += snprintf(buf + pos, LEN_OR_ZERO, " %d", field->size);
+
 	if (colon)
 		pos += snprintf(buf + pos, LEN_OR_ZERO, ";");
 
Signature · 1 line
2.34.1
#2 lore raw

PATCH 2/2 selftests/user_events: Test struct size match cases

In reply to Beau Belgrave

The self tests for user_events currently does not ensure that the edge case for struct types work properly with size differences.

Add cases for mis-matching struct names and sizes to ensure they work properly.

---
 tools/testing/selftests/user_events/dyn_test.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)
tools/testing/selftests/user_events/dyn_test.c +12 -0
index d6979a48478f..91a4444ad42b 100644
@@ -217,6 +217,18 @@ TEST_F(user, matching) {
 	/* Types don't match */
 	TEST_NMATCH("__test_event u64 a; u64 b",
 		    "__test_event u32 a; u32 b");
+
+	/* Struct name and size matches */
+	TEST_MATCH("__test_event struct my_struct a 20",
+		   "__test_event struct my_struct a 20");
+
+	/* Struct name don't match */
+	TEST_NMATCH("__test_event struct my_struct a 20",
+		    "__test_event struct my_struct b 20");
+
+	/* Struct size don't match */
+	TEST_NMATCH("__test_event struct my_struct a 20",
+		    "__test_event struct my_struct a 21");
 }
 
 int main(int argc, char **argv)
Signature · 1 line
2.34.1