Skip to content

Commit

Permalink
tracing: Show real address for trace event arguments
Browse files Browse the repository at this point in the history
To help debugging kernel, show real address for trace event arguments
in tracefs/trace{,pipe} instead of hashed pointer value.

Since ftrace human-readable format uses vsprintf(), all %p are
translated to hash values instead of pointer address.

However, when debugging the kernel, raw address value gives a
hint when comparing with the memory mapping in the kernel.
(Those are sometimes used with crash log, which is not hashed too)
So converting %p with %px when calling trace_seq_printf().

Moreover, this is not improving the security because the tracefs
can be used only by root user and the raw address values are readable
from tracefs/percpu/cpu*/trace_pipe_raw file.

Link: https://lkml.kernel.org/r/160277370703.29307.5134475491761971203.stgit@devnote2

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
  • Loading branch information
Masami Hiramatsu authored and Steven Rostedt (VMware) committed Feb 11, 2021
1 parent 7d53675 commit efbbdaa
Show file tree
Hide file tree
Showing 5 changed files with 88 additions and 3 deletions.
4 changes: 4 additions & 0 deletions include/linux/trace_events.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ struct trace_event;

int trace_raw_output_prep(struct trace_iterator *iter,
struct trace_event *event);
extern __printf(2, 3)
void trace_event_printf(struct trace_iterator *iter, const char *fmt, ...);

/*
* The trace entry - the most basic unit of tracing. This is what
Expand Down Expand Up @@ -87,6 +89,8 @@ struct trace_iterator {
unsigned long iter_flags;
void *temp; /* temp holder */
unsigned int temp_size;
char *fmt; /* modified format holder */
unsigned int fmt_size;

/* trace_seq for __print_flags() and __print_symbolic() etc. */
struct trace_seq tmp_seq;
Expand Down
2 changes: 1 addition & 1 deletion include/trace/trace_events.h
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ trace_raw_output_##call(struct trace_iterator *iter, int flags, \
if (ret != TRACE_TYPE_HANDLED) \
return ret; \
\
trace_seq_printf(s, print); \
trace_event_printf(iter, print); \
\
return trace_handle_return(s); \
} \
Expand Down
71 changes: 70 additions & 1 deletion kernel/trace/trace.c
Original file line number Diff line number Diff line change
Expand Up @@ -3530,6 +3530,62 @@ __find_next_entry(struct trace_iterator *iter, int *ent_cpu,
return next;
}

#define STATIC_FMT_BUF_SIZE 128
static char static_fmt_buf[STATIC_FMT_BUF_SIZE];

static char *trace_iter_expand_format(struct trace_iterator *iter)
{
char *tmp;

if (iter->fmt == static_fmt_buf)
return NULL;

tmp = krealloc(iter->fmt, iter->fmt_size + STATIC_FMT_BUF_SIZE,
GFP_KERNEL);
if (tmp) {
iter->fmt_size += STATIC_FMT_BUF_SIZE;
iter->fmt = tmp;
}

return tmp;
}

const char *trace_event_format(struct trace_iterator *iter, const char *fmt)
{
const char *p, *new_fmt;
char *q;

if (WARN_ON_ONCE(!fmt))
return fmt;

p = fmt;
new_fmt = q = iter->fmt;
while (*p) {
if (unlikely(q - new_fmt + 3 > iter->fmt_size)) {
if (!trace_iter_expand_format(iter))
return fmt;

q += iter->fmt - new_fmt;
new_fmt = iter->fmt;
}

*q++ = *p++;

/* Replace %p with %px */
if (p[-1] == '%') {
if (p[0] == '%') {
*q++ = *p++;
} else if (p[0] == 'p' && !isalnum(p[1])) {
*q++ = *p++;
*q++ = 'x';
}
}
}
*q = '\0';

return new_fmt;
}

#define STATIC_TEMP_BUF_SIZE 128
static char static_temp_buf[STATIC_TEMP_BUF_SIZE] __aligned(4);

Expand Down Expand Up @@ -4322,6 +4378,16 @@ __tracing_open(struct inode *inode, struct file *file, bool snapshot)
if (iter->temp)
iter->temp_size = 128;

/*
* trace_event_printf() may need to modify given format
* string to replace %p with %px so that it shows real address
* instead of hash value. However, that is only for the event
* tracing, other tracer may not need. Defer the allocation
* until it is needed.
*/
iter->fmt = NULL;
iter->fmt_size = 0;

/*
* We make a copy of the current tracer to avoid concurrent
* changes on it while we are reading.
Expand Down Expand Up @@ -4473,6 +4539,7 @@ static int tracing_release(struct inode *inode, struct file *file)

mutex_destroy(&iter->mutex);
free_cpumask_var(iter->started);
kfree(iter->fmt);
kfree(iter->temp);
kfree(iter->trace);
kfree(iter->buffer_iter);
Expand Down Expand Up @@ -9331,9 +9398,11 @@ void ftrace_dump(enum ftrace_dump_mode oops_dump_mode)

/* Simulate the iterator */
trace_init_global_iter(&iter);
/* Can not use kmalloc for iter.temp */
/* Can not use kmalloc for iter.temp and iter.fmt */
iter.temp = static_temp_buf;
iter.temp_size = STATIC_TEMP_BUF_SIZE;
iter.fmt = static_fmt_buf;
iter.fmt_size = STATIC_FMT_BUF_SIZE;

for_each_tracing_cpu(cpu) {
atomic_inc(&per_cpu_ptr(iter.array_buffer->data, cpu)->disabled);
Expand Down
2 changes: 2 additions & 0 deletions kernel/trace/trace.h
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,8 @@ struct trace_entry *trace_find_next_entry(struct trace_iterator *iter,
void trace_buffer_unlock_commit_nostack(struct trace_buffer *buffer,
struct ring_buffer_event *event);

const char *trace_event_format(struct trace_iterator *iter, const char *fmt);

int trace_empty(struct trace_iterator *iter);

void *trace_find_next_entry_inc(struct trace_iterator *iter);
Expand Down
12 changes: 11 additions & 1 deletion kernel/trace/trace_output.c
Original file line number Diff line number Diff line change
Expand Up @@ -312,13 +312,23 @@ int trace_raw_output_prep(struct trace_iterator *iter,
}
EXPORT_SYMBOL(trace_raw_output_prep);

void trace_event_printf(struct trace_iterator *iter, const char *fmt, ...)
{
va_list ap;

va_start(ap, fmt);
trace_seq_vprintf(&iter->seq, trace_event_format(iter, fmt), ap);
va_end(ap);
}
EXPORT_SYMBOL(trace_event_printf);

static int trace_output_raw(struct trace_iterator *iter, char *name,
char *fmt, va_list ap)
{
struct trace_seq *s = &iter->seq;

trace_seq_printf(s, "%s: ", name);
trace_seq_vprintf(s, fmt, ap);
trace_seq_vprintf(s, trace_event_format(iter, fmt), ap);

return trace_handle_return(s);
}
Expand Down

0 comments on commit efbbdaa

Please sign in to comment.