From 17b439db21d5dbe70c419e982262621e5e6aba7f Mon Sep 17 00:00:00 2001 From: Beau Belgrave Date: Tue, 25 Apr 2023 15:51:05 -0700 Subject: [PATCH] tracing/user_events: Ensure bit is cleared on unregister If an event is enabled and a user process unregisters user_events, the bit is left set. Fix this by always clearing the bit in the user process if unregister is successful. Update abi self-test to ensure this occurs properly. Link: https://lkml.kernel.org/r/20230425225107.8525-3-beaub@linux.microsoft.com Suggested-by: Doug Cook Signed-off-by: Beau Belgrave Signed-off-by: Steven Rostedt (Google) --- kernel/trace/trace_events_user.c | 34 ++++++++++++++++++++++++++ tools/testing/selftests/user_events/abi_test.c | 9 ++++--- 2 files changed, 40 insertions(+), 3 deletions(-) diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c index 546d47a..4f9ae63 100644 --- a/kernel/trace/trace_events_user.c +++ b/kernel/trace/trace_events_user.c @@ -2149,6 +2149,35 @@ static long user_unreg_get(struct user_unreg __user *ureg, return ret; } +static int user_event_mm_clear_bit(struct user_event_mm *user_mm, + unsigned long uaddr, unsigned char bit) +{ + struct user_event_enabler enabler; + int result; + + memset(&enabler, 0, sizeof(enabler)); + enabler.addr = uaddr; + enabler.values = bit; +retry: + /* Prevents state changes from racing with new enablers */ + mutex_lock(&event_mutex); + + /* Force the bit to be cleared, since no event is attached */ + mmap_read_lock(user_mm->mm); + result = user_event_enabler_write(user_mm, &enabler, false); + mmap_read_unlock(user_mm->mm); + + mutex_unlock(&event_mutex); + + if (result) { + /* Attempt to fault-in and retry if it worked */ + if (!user_event_mm_fault_in(user_mm, uaddr)) + goto retry; + } + + return result; +} + /* * Unregisters an enablement address/bit within a task/user mm. */ @@ -2193,6 +2222,11 @@ static long user_events_ioctl_unreg(unsigned long uarg) mutex_unlock(&event_mutex); + /* Ensure bit is now cleared for user, regardless of event status */ + if (!ret) + ret = user_event_mm_clear_bit(mm, reg.disable_addr, + reg.disable_bit); + return ret; } diff --git a/tools/testing/selftests/user_events/abi_test.c b/tools/testing/selftests/user_events/abi_test.c index e0323d3..5125c42 100644 --- a/tools/testing/selftests/user_events/abi_test.c +++ b/tools/testing/selftests/user_events/abi_test.c @@ -109,13 +109,16 @@ TEST_F(user, enablement) { ASSERT_EQ(0, change_event(false)); ASSERT_EQ(0, self->check); - /* Should not change after disable */ + /* Ensure kernel clears bit after disable */ ASSERT_EQ(0, change_event(true)); ASSERT_EQ(1, self->check); ASSERT_EQ(0, reg_disable(&self->check, 0)); + ASSERT_EQ(0, self->check); + + /* Ensure doesn't change after unreg */ + ASSERT_EQ(0, change_event(true)); + ASSERT_EQ(0, self->check); ASSERT_EQ(0, change_event(false)); - ASSERT_EQ(1, self->check); - self->check = 0; } TEST_F(user, bit_sizes) { -- 2.7.4