From: Steven Rostedt <rostedt@goodmis.org>
To: linux-kernel@vger.kernel.org
Cc: Ingo Molnar <mingo@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Beau Belgrave <beaub@linux.microsoft.com>
Subject: [for-next][PATCH 12/15] tracing/user_events: Ensure user provided strings are safely formatted
Date: Thu, 29 Sep 2022 18:55:54 -0400 [thread overview]
Message-ID: <20220929225639.943545555@goodmis.org> (raw)
In-Reply-To: 20220929225542.784716766@goodmis.org
From: Beau Belgrave <beaub@linux.microsoft.com>
User processes can provide bad strings that may cause issues or leak
kernel details back out. Don't trust the content of these strings
when formatting strings for matching.
This also moves to a consistent dynamic length string creation model.
Link: https://lkml.kernel.org/r/20220728233309.1896-4-beaub@linux.microsoft.com
Link: https://lore.kernel.org/all/2059213643.196683.1648499088753.JavaMail.zimbra@efficios.com/
Reported-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Signed-off-by: Beau Belgrave <beaub@linux.microsoft.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
kernel/trace/trace_events_user.c | 91 +++++++++++++++++++++-----------
1 file changed, 59 insertions(+), 32 deletions(-)
diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
index 15edbf6b1e2e..f9bb7d37d76f 100644
--- a/kernel/trace/trace_events_user.c
+++ b/kernel/trace/trace_events_user.c
@@ -45,7 +45,6 @@
#define MAX_EVENT_DESC 512
#define EVENT_NAME(user_event) ((user_event)->tracepoint.name)
#define MAX_FIELD_ARRAY_SIZE 1024
-#define MAX_FIELD_ARG_NAME 256
static char *register_page_data;
@@ -483,6 +482,48 @@ static bool user_field_is_dyn_string(const char *type, const char **str_func)
}
#define LEN_OR_ZERO (len ? len - pos : 0)
+static int user_dyn_field_set_string(int argc, const char **argv, int *iout,
+ char *buf, int len, bool *colon)
+{
+ int pos = 0, i = *iout;
+
+ *colon = false;
+
+ for (; i < argc; ++i) {
+ if (i != *iout)
+ pos += snprintf(buf + pos, LEN_OR_ZERO, " ");
+
+ pos += snprintf(buf + pos, LEN_OR_ZERO, "%s", argv[i]);
+
+ if (strchr(argv[i], ';')) {
+ ++i;
+ *colon = true;
+ break;
+ }
+ }
+
+ /* Actual set, advance i */
+ if (len != 0)
+ *iout = i;
+
+ return pos + 1;
+}
+
+static int user_field_set_string(struct ftrace_event_field *field,
+ char *buf, int len, bool colon)
+{
+ int pos = 0;
+
+ pos += snprintf(buf + pos, LEN_OR_ZERO, "%s", field->type);
+ pos += snprintf(buf + pos, LEN_OR_ZERO, " ");
+ pos += snprintf(buf + pos, LEN_OR_ZERO, "%s", field->name);
+
+ if (colon)
+ pos += snprintf(buf + pos, LEN_OR_ZERO, ";");
+
+ return pos + 1;
+}
+
static int user_event_set_print_fmt(struct user_event *user, char *buf, int len)
{
struct ftrace_event_field *field, *next;
@@ -926,49 +967,35 @@ static int user_event_free(struct dyn_event *ev)
static bool user_field_match(struct ftrace_event_field *field, int argc,
const char **argv, int *iout)
{
- char *field_name, *arg_name;
- int len, pos, i = *iout;
+ char *field_name = NULL, *dyn_field_name = NULL;
bool colon = false, match = false;
+ int dyn_len, len;
- if (i >= argc)
+ if (*iout >= argc)
return false;
- len = MAX_FIELD_ARG_NAME;
- field_name = kmalloc(len, GFP_KERNEL);
- arg_name = kmalloc(len, GFP_KERNEL);
+ dyn_len = user_dyn_field_set_string(argc, argv, iout, dyn_field_name,
+ 0, &colon);
- if (!arg_name || !field_name)
- goto out;
-
- pos = 0;
-
- for (; i < argc; ++i) {
- if (i != *iout)
- pos += snprintf(arg_name + pos, len - pos, " ");
+ len = user_field_set_string(field, field_name, 0, colon);
- pos += snprintf(arg_name + pos, len - pos, argv[i]);
-
- if (strchr(argv[i], ';')) {
- ++i;
- colon = true;
- break;
- }
- }
+ if (dyn_len != len)
+ return false;
- pos = 0;
+ dyn_field_name = kmalloc(dyn_len, GFP_KERNEL);
+ field_name = kmalloc(len, GFP_KERNEL);
- pos += snprintf(field_name + pos, len - pos, field->type);
- pos += snprintf(field_name + pos, len - pos, " ");
- pos += snprintf(field_name + pos, len - pos, field->name);
+ if (!dyn_field_name || !field_name)
+ goto out;
- if (colon)
- pos += snprintf(field_name + pos, len - pos, ";");
+ user_dyn_field_set_string(argc, argv, iout, dyn_field_name,
+ dyn_len, &colon);
- *iout = i;
+ user_field_set_string(field, field_name, len, colon);
- match = strcmp(arg_name, field_name) == 0;
+ match = strcmp(dyn_field_name, field_name) == 0;
out:
- kfree(arg_name);
+ kfree(dyn_field_name);
kfree(field_name);
return match;
--
2.35.1
next prev parent reply other threads:[~2022-09-29 22:56 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-09-29 22:55 [for-next][PATCH 00/15] tracing: More updates for 6.1 Steven Rostedt
2022-09-29 22:55 ` [for-next][PATCH 01/15] ftrace: Fix recursive locking direct_mutex in ftrace_modify_direct_caller Steven Rostedt
2022-09-29 22:55 ` [for-next][PATCH 02/15] ring-buffer: Allow splice to read previous partially read pages Steven Rostedt
2022-09-29 22:55 ` [for-next][PATCH 03/15] ring-buffer: Have the shortest_full queue be the shortest not longest Steven Rostedt
2022-09-29 22:55 ` [for-next][PATCH 04/15] ring-buffer: Check pending waiters when doing wake ups as well Steven Rostedt
2022-09-29 22:55 ` [for-next][PATCH 05/15] ring-buffer: Add ring_buffer_wake_waiters() Steven Rostedt
2022-09-29 22:55 ` [for-next][PATCH 06/15] tracing: Wake up ring buffer waiters on closing of the file Steven Rostedt
2022-09-29 22:55 ` [for-next][PATCH 07/15] tracing: Add ioctl() to force ring buffer waiters to wake up Steven Rostedt
2022-09-29 22:55 ` [for-next][PATCH 08/15] tracing: Wake up waiters when tracing is disabled Steven Rostedt
2022-09-29 22:55 ` [for-next][PATCH 09/15] tracing: Fix spelling mistake "preapre" -> "prepare" Steven Rostedt
2022-09-29 22:55 ` [for-next][PATCH 10/15] tracing/user_events: Use NULL for strstr checks Steven Rostedt
2022-09-29 22:55 ` [for-next][PATCH 11/15] tracing/user_events: Use WRITE instead of READ for io vector import Steven Rostedt
2022-09-29 22:55 ` Steven Rostedt [this message]
2022-09-29 22:55 ` [for-next][PATCH 13/15] tracing/user_events: Use refcount instead of atomic for ref tracking Steven Rostedt
2022-09-29 22:55 ` [for-next][PATCH 14/15] tracing/user_events: Use bits vs bytes for enabled status page data Steven Rostedt
2022-09-29 22:55 ` [for-next][PATCH 15/15] tracing/user_events: Update ABI documentation to align to bits vs bytes 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=20220929225639.943545555@goodmis.org \
--to=rostedt@goodmis.org \
--cc=akpm@linux-foundation.org \
--cc=beaub@linux.microsoft.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mingo@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