Skip to content

Commit

Permalink
tracing/user_events: Ensure user provided strings are safely formatted
Browse files Browse the repository at this point in the history
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/[email protected]
Link: https://lore.kernel.org/all/[email protected]/

Reported-by: Mathieu Desnoyers <[email protected]>
Signed-off-by: Beau Belgrave <[email protected]>
Signed-off-by: Steven Rostedt (Google) <[email protected]>
  • Loading branch information
beaubelgrave authored and rostedt committed Sep 29, 2022
1 parent 95f1876 commit e6f89a1
Showing 1 changed file with 59 additions and 32 deletions.
91 changes: 59 additions & 32 deletions kernel/trace/trace_events_user.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit e6f89a1

Please sign in to comment.