From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Andrew Morton <akpm@linux-foundation.org>,
Beau Belgrave <beaub@linux.microsoft.com>
Subject: [for-next][PATCH 08/11] tracing/user_events: Limit max fault-in attempts
Date: Wed, 26 Apr 2023 13:17:11 -0400 [thread overview]
Message-ID: <20230426171750.635695746@goodmis.org> (raw)
In-Reply-To: <20230426171703.202523909@goodmis.org>
From: Beau Belgrave <beaub@linux.microsoft.com>
When event enablement changes, user_events attempts to update a bit in
the user process. If a fault is hit, an attempt to fault-in the page and
the write is retried if the page made it in. While this normally requires
a couple attempts, it is possible a bad user process could attempt to
cause infinite loops.
Ensure fault-in attempts either sync or async are limited to a max of 10
attempts for each update. When the max is hit, return -EFAULT so another
attempt is not made in all cases.
Link: https://lkml.kernel.org/r/20230425225107.8525-5-beaub@linux.microsoft.com
Suggested-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Beau Belgrave <beaub@linux.microsoft.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_events_user.c | 49 +++++++++++++++++++++++---------
1 file changed, 35 insertions(+), 14 deletions(-)
diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
index a29cd13caf55..b1ecd7677642 100644
--- a/kernel/trace/trace_events_user.c
+++ b/kernel/trace/trace_events_user.c
@@ -123,6 +123,7 @@ struct user_event_enabler_fault {
struct work_struct work;
struct user_event_mm *mm;
struct user_event_enabler *enabler;
+ int attempt;
};
static struct kmem_cache *fault_cache;
@@ -266,11 +267,19 @@ static void user_event_enabler_destroy(struct user_event_enabler *enabler)
kfree(enabler);
}
-static int user_event_mm_fault_in(struct user_event_mm *mm, unsigned long uaddr)
+static int user_event_mm_fault_in(struct user_event_mm *mm, unsigned long uaddr,
+ int attempt)
{
bool unlocked;
int ret;
+ /*
+ * Normally this is low, ensure that it cannot be taken advantage of by
+ * bad user processes to cause excessive looping.
+ */
+ if (attempt > 10)
+ return -EFAULT;
+
mmap_read_lock(mm->mm);
/* Ensure MM has tasks, cannot use after exit_mm() */
@@ -289,7 +298,7 @@ static int user_event_mm_fault_in(struct user_event_mm *mm, unsigned long uaddr)
static int user_event_enabler_write(struct user_event_mm *mm,
struct user_event_enabler *enabler,
- bool fixup_fault);
+ bool fixup_fault, int *attempt);
static void user_event_enabler_fault_fixup(struct work_struct *work)
{
@@ -298,9 +307,10 @@ static void user_event_enabler_fault_fixup(struct work_struct *work)
struct user_event_enabler *enabler = fault->enabler;
struct user_event_mm *mm = fault->mm;
unsigned long uaddr = enabler->addr;
+ int attempt = fault->attempt;
int ret;
- ret = user_event_mm_fault_in(mm, uaddr);
+ ret = user_event_mm_fault_in(mm, uaddr, attempt);
if (ret && ret != -ENOENT) {
struct user_event *user = enabler->event;
@@ -329,7 +339,7 @@ static void user_event_enabler_fault_fixup(struct work_struct *work)
if (!ret) {
mmap_read_lock(mm->mm);
- user_event_enabler_write(mm, enabler, true);
+ user_event_enabler_write(mm, enabler, true, &attempt);
mmap_read_unlock(mm->mm);
}
out:
@@ -341,7 +351,8 @@ static void user_event_enabler_fault_fixup(struct work_struct *work)
}
static bool user_event_enabler_queue_fault(struct user_event_mm *mm,
- struct user_event_enabler *enabler)
+ struct user_event_enabler *enabler,
+ int attempt)
{
struct user_event_enabler_fault *fault;
@@ -353,6 +364,7 @@ static bool user_event_enabler_queue_fault(struct user_event_mm *mm,
INIT_WORK(&fault->work, user_event_enabler_fault_fixup);
fault->mm = user_event_mm_get(mm);
fault->enabler = enabler;
+ fault->attempt = attempt;
/* Don't try to queue in again while we have a pending fault */
set_bit(ENABLE_VAL_FAULTING_BIT, ENABLE_BITOPS(enabler));
@@ -372,7 +384,7 @@ static bool user_event_enabler_queue_fault(struct user_event_mm *mm,
static int user_event_enabler_write(struct user_event_mm *mm,
struct user_event_enabler *enabler,
- bool fixup_fault)
+ bool fixup_fault, int *attempt)
{
unsigned long uaddr = enabler->addr;
unsigned long *ptr;
@@ -383,6 +395,8 @@ static int user_event_enabler_write(struct user_event_mm *mm,
lockdep_assert_held(&event_mutex);
mmap_assert_locked(mm->mm);
+ *attempt += 1;
+
/* Ensure MM has tasks, cannot use after exit_mm() */
if (refcount_read(&mm->tasks) == 0)
return -ENOENT;
@@ -398,7 +412,7 @@ static int user_event_enabler_write(struct user_event_mm *mm,
if (!fixup_fault)
return -EFAULT;
- if (!user_event_enabler_queue_fault(mm, enabler))
+ if (!user_event_enabler_queue_fault(mm, enabler, *attempt))
pr_warn("user_events: Unable to queue fault handler\n");
return -EFAULT;
@@ -439,15 +453,19 @@ static void user_event_enabler_update(struct user_event *user)
struct user_event_enabler *enabler;
struct user_event_mm *mm = user_event_mm_get_all(user);
struct user_event_mm *next;
+ int attempt;
while (mm) {
next = mm->next;
mmap_read_lock(mm->mm);
rcu_read_lock();
- list_for_each_entry_rcu(enabler, &mm->enablers, link)
- if (enabler->event == user)
- user_event_enabler_write(mm, enabler, true);
+ list_for_each_entry_rcu(enabler, &mm->enablers, link) {
+ if (enabler->event == user) {
+ attempt = 0;
+ user_event_enabler_write(mm, enabler, true, &attempt);
+ }
+ }
rcu_read_unlock();
mmap_read_unlock(mm->mm);
@@ -695,6 +713,7 @@ static struct user_event_enabler
struct user_event_enabler *enabler;
struct user_event_mm *user_mm;
unsigned long uaddr = (unsigned long)reg->enable_addr;
+ int attempt = 0;
user_mm = current_user_event_mm();
@@ -715,7 +734,8 @@ static struct user_event_enabler
/* Attempt to reflect the current state within the process */
mmap_read_lock(user_mm->mm);
- *write_result = user_event_enabler_write(user_mm, enabler, false);
+ *write_result = user_event_enabler_write(user_mm, enabler, false,
+ &attempt);
mmap_read_unlock(user_mm->mm);
/*
@@ -735,7 +755,7 @@ static struct user_event_enabler
if (*write_result) {
/* Attempt to fault-in and retry if it worked */
- if (!user_event_mm_fault_in(user_mm, uaddr))
+ if (!user_event_mm_fault_in(user_mm, uaddr, attempt))
goto retry;
kfree(enabler);
@@ -2195,6 +2215,7 @@ static int user_event_mm_clear_bit(struct user_event_mm *user_mm,
{
struct user_event_enabler enabler;
int result;
+ int attempt = 0;
memset(&enabler, 0, sizeof(enabler));
enabler.addr = uaddr;
@@ -2205,14 +2226,14 @@ static int user_event_mm_clear_bit(struct user_event_mm *user_mm,
/* 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);
+ result = user_event_enabler_write(user_mm, &enabler, false, &attempt);
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))
+ if (!user_event_mm_fault_in(user_mm, uaddr, attempt))
goto retry;
}
--
2.39.2
next prev parent reply other threads:[~2023-04-26 17:18 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-04-26 17:17 [for-next][PATCH 00/11] tracing: Final fixes and minor updates before sending pull request Steven Rostedt
2023-04-26 17:17 ` [for-next][PATCH 01/11] ring-buffer: Clearly check null ptr returned by rb_set_head_page() Steven Rostedt
2023-04-26 17:17 ` [for-next][PATCH 02/11] tracing/user_events: Set event filter_type from type Steven Rostedt
2023-04-26 17:17 ` [for-next][PATCH 03/11] tracing: Fix print_fields() for __dyn_loc/__rel_loc Steven Rostedt
2023-04-26 17:17 ` [for-next][PATCH 04/11] seq_buf: Add seq_buf_do_printk() helper Steven Rostedt
2023-04-26 17:17 ` [for-next][PATCH 05/11] tracing/user_events: Ensure write index cannot be negative Steven Rostedt
2023-04-26 17:17 ` [for-next][PATCH 06/11] tracing/user_events: Ensure bit is cleared on unregister Steven Rostedt
2023-04-26 17:17 ` [for-next][PATCH 07/11] tracing/user_events: Prevent same address and bit per process Steven Rostedt
2023-04-26 17:17 ` Steven Rostedt [this message]
2023-04-26 17:17 ` [for-next][PATCH 09/11] recordmcount: Fix memory leaks in the uwrite function Steven Rostedt
2023-04-26 17:17 ` [for-next][PATCH 10/11] ring-buffer: Ensure proper resetting of atomic variables in ring_buffer_reset_online_cpus Steven Rostedt
2023-04-26 17:17 ` [for-next][PATCH 11/11] tracing: Add missing spaces in trace_print_hex_seq() Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230426171750.635695746@goodmis.org \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=beaub@linux.microsoft.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mhiramat@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
Powered by JetHome