Skip to content

Commit ba470ee

Browse files
sunlimingrostedt
authored andcommitted
tracing/user_events: Prevent same name but different args event
User processes register name_args for events. If the same name but different args event are registered. The trace outputs of second event are printed as the first event. This is incorrect. Return EADDRINUSE back to the user process if the same name but different args event has being registered. Link: https://lore.kernel.org/linux-trace-kernel/[email protected] Signed-off-by: sunliming <[email protected]> Reviewed-by: Masami Hiramatsu (Google) <[email protected]> Acked-by: Beau Belgrave <[email protected]> Signed-off-by: Steven Rostedt (Google) <[email protected]>
1 parent aafbb1e commit ba470ee

File tree

2 files changed

+36
-6
lines changed

2 files changed

+36
-6
lines changed

kernel/trace/trace_events_user.c

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1786,20 +1786,44 @@ static int user_event_parse(struct user_event_group *group, char *name,
17861786
int ret;
17871787
u32 key;
17881788
struct user_event *user;
1789+
int argc = 0;
1790+
char **argv;
17891791

17901792
/* Prevent dyn_event from racing */
17911793
mutex_lock(&event_mutex);
17921794
user = find_user_event(group, name, &key);
17931795
mutex_unlock(&event_mutex);
17941796

17951797
if (user) {
1796-
*newuser = user;
1797-
/*
1798-
* Name is allocated by caller, free it since it already exists.
1799-
* Caller only worries about failure cases for freeing.
1800-
*/
1801-
kfree(name);
1798+
if (args) {
1799+
argv = argv_split(GFP_KERNEL, args, &argc);
1800+
if (!argv) {
1801+
ret = -ENOMEM;
1802+
goto error;
1803+
}
1804+
1805+
ret = user_fields_match(user, argc, (const char **)argv);
1806+
argv_free(argv);
1807+
1808+
} else
1809+
ret = list_empty(&user->fields);
1810+
1811+
if (ret) {
1812+
*newuser = user;
1813+
/*
1814+
* Name is allocated by caller, free it since it already exists.
1815+
* Caller only worries about failure cases for freeing.
1816+
*/
1817+
kfree(name);
1818+
} else {
1819+
ret = -EADDRINUSE;
1820+
goto error;
1821+
}
1822+
18021823
return 0;
1824+
error:
1825+
refcount_dec(&user->refcnt);
1826+
return ret;
18031827
}
18041828

18051829
user = kzalloc(sizeof(*user), GFP_KERNEL_ACCOUNT);

tools/testing/selftests/user_events/ftrace_test.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,12 @@ TEST_F(user, register_events) {
228228
ASSERT_EQ(0, ioctl(self->data_fd, DIAG_IOCSREG, &reg));
229229
ASSERT_EQ(0, reg.write_index);
230230

231+
/* Multiple registers to same name but different args should fail */
232+
reg.enable_bit = 29;
233+
reg.name_args = (__u64)"__test_event u32 field1;";
234+
ASSERT_EQ(-1, ioctl(self->data_fd, DIAG_IOCSREG, &reg));
235+
ASSERT_EQ(EADDRINUSE, errno);
236+
231237
/* Ensure disabled */
232238
self->enable_fd = open(enable_file, O_RDWR);
233239
ASSERT_NE(-1, self->enable_fd);

0 commit comments

Comments
 (0)