Skip to content

Commit

Permalink
tracing: protect reader of cmdline output
Browse files Browse the repository at this point in the history
Impact: fix to one cause of incorrect comm outputs in trace

The spinlock only protected the creation of a comm <=> pid pair.
But it was possible that a reader could look up a pid, and get the
wrong comm because it had no locking.

This also required changing trace_find_cmdline to copy the comm cache
and not just send back a pointer to it.

Signed-off-by: Steven Rostedt <srostedt@redhat.com>
  • Loading branch information
Steven Rostedt committed Mar 17, 2009
1 parent 0330354 commit 4ca5308
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 26 deletions.
23 changes: 18 additions & 5 deletions kernel/trace/blktrace.c
Original file line number Diff line number Diff line change
Expand Up @@ -1027,7 +1027,9 @@ static int blk_log_action_seq(struct trace_seq *s, const struct blk_io_trace *t,

static int blk_log_generic(struct trace_seq *s, const struct trace_entry *ent)
{
const char *cmd = trace_find_cmdline(ent->pid);
char cmd[TASK_COMM_LEN];

trace_find_cmdline(ent->pid, cmd);

if (t_sec(ent))
return trace_seq_printf(s, "%llu + %u [%s]\n",
Expand Down Expand Up @@ -1057,19 +1059,30 @@ static int blk_log_remap(struct trace_seq *s, const struct trace_entry *ent)

static int blk_log_plug(struct trace_seq *s, const struct trace_entry *ent)
{
return trace_seq_printf(s, "[%s]\n", trace_find_cmdline(ent->pid));
char cmd[TASK_COMM_LEN];

trace_find_cmdline(ent->pid, cmd);

return trace_seq_printf(s, "[%s]\n", cmd);
}

static int blk_log_unplug(struct trace_seq *s, const struct trace_entry *ent)
{
return trace_seq_printf(s, "[%s] %llu\n", trace_find_cmdline(ent->pid),
get_pdu_int(ent));
char cmd[TASK_COMM_LEN];

trace_find_cmdline(ent->pid, cmd);

return trace_seq_printf(s, "[%s] %llu\n", cmd, get_pdu_int(ent));
}

static int blk_log_split(struct trace_seq *s, const struct trace_entry *ent)
{
char cmd[TASK_COMM_LEN];

trace_find_cmdline(ent->pid, cmd);

return trace_seq_printf(s, "%llu / %llu [%s]\n", t_sector(ent),
get_pdu_int(ent), trace_find_cmdline(ent->pid));
get_pdu_int(ent), cmd);
}

/*
Expand Down
20 changes: 12 additions & 8 deletions kernel/trace/trace.c
Original file line number Diff line number Diff line change
Expand Up @@ -770,25 +770,29 @@ static void trace_save_cmdline(struct task_struct *tsk)
__raw_spin_unlock(&trace_cmdline_lock);
}

char *trace_find_cmdline(int pid)
void trace_find_cmdline(int pid, char comm[])
{
char *cmdline = "<...>";
unsigned map;

if (!pid)
return "<idle>";
if (!pid) {
strcpy(comm, "<idle>");
return;
}

if (pid > PID_MAX_DEFAULT)
goto out;
if (pid > PID_MAX_DEFAULT) {
strcpy(comm, "<...>");
return;
}

__raw_spin_lock(&trace_cmdline_lock);
map = map_pid_to_cmdline[pid];
if (map >= SAVED_CMDLINES)
goto out;

cmdline = saved_cmdlines[map];
strcpy(comm, saved_cmdlines[map]);

out:
return cmdline;
__raw_spin_unlock(&trace_cmdline_lock);
}

void tracing_record_cmdline(struct task_struct *tsk)
Expand Down
2 changes: 1 addition & 1 deletion kernel/trace/trace.h
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ struct tracer_switch_ops {
};
#endif /* CONFIG_CONTEXT_SWITCH_TRACER */

extern char *trace_find_cmdline(int pid);
extern void trace_find_cmdline(int pid, char comm[]);

#ifdef CONFIG_DYNAMIC_FTRACE
extern unsigned long ftrace_update_tot_cnt;
Expand Down
12 changes: 6 additions & 6 deletions kernel/trace/trace_functions_graph.c
Original file line number Diff line number Diff line change
Expand Up @@ -190,15 +190,15 @@ print_graph_cpu(struct trace_seq *s, int cpu)
static enum print_line_t
print_graph_proc(struct trace_seq *s, pid_t pid)
{
int i;
int ret;
int len;
char comm[8];
int spaces = 0;
char comm[TASK_COMM_LEN];
/* sign + log10(MAX_INT) + '\0' */
char pid_str[11];
int spaces = 0;
int ret;
int len;
int i;

strncpy(comm, trace_find_cmdline(pid), 7);
trace_find_cmdline(pid, comm);
comm[7] = '\0';
sprintf(pid_str, "%d", pid);

Expand Down
18 changes: 12 additions & 6 deletions kernel/trace/trace_output.c
Original file line number Diff line number Diff line change
Expand Up @@ -309,9 +309,9 @@ static int
lat_print_generic(struct trace_seq *s, struct trace_entry *entry, int cpu)
{
int hardirq, softirq;
char *comm;
char comm[TASK_COMM_LEN];

comm = trace_find_cmdline(entry->pid);
trace_find_cmdline(entry->pid, comm);
hardirq = entry->flags & TRACE_FLAG_HARDIRQ;
softirq = entry->flags & TRACE_FLAG_SOFTIRQ;

Expand Down Expand Up @@ -346,10 +346,12 @@ int trace_print_context(struct trace_iterator *iter)
{
struct trace_seq *s = &iter->seq;
struct trace_entry *entry = iter->ent;
char *comm = trace_find_cmdline(entry->pid);
unsigned long long t = ns2usecs(iter->ts);
unsigned long usec_rem = do_div(t, USEC_PER_SEC);
unsigned long secs = (unsigned long)t;
char comm[TASK_COMM_LEN];

trace_find_cmdline(entry->pid, comm);

return trace_seq_printf(s, "%16s-%-5d [%03d] %5lu.%06lu: ",
comm, entry->pid, iter->cpu, secs, usec_rem);
Expand All @@ -372,7 +374,10 @@ int trace_print_lat_context(struct trace_iterator *iter)
rel_usecs = ns2usecs(next_ts - iter->ts);

if (verbose) {
char *comm = trace_find_cmdline(entry->pid);
char comm[TASK_COMM_LEN];

trace_find_cmdline(entry->pid, comm);

ret = trace_seq_printf(s, "%16s %5d %3d %d %08x %08lx [%08lx]"
" %ld.%03ldms (+%ld.%03ldms): ", comm,
entry->pid, iter->cpu, entry->flags,
Expand Down Expand Up @@ -577,14 +582,15 @@ static enum print_line_t trace_ctxwake_print(struct trace_iterator *iter,
char *delim)
{
struct ctx_switch_entry *field;
char *comm;
char comm[TASK_COMM_LEN];
int S, T;


trace_assign_type(field, iter->ent);

T = task_state_char(field->next_state);
S = task_state_char(field->prev_state);
comm = trace_find_cmdline(field->next_pid);
trace_find_cmdline(field->next_pid, comm);
if (!trace_seq_printf(&iter->seq,
" %5d:%3d:%c %s [%03d] %5d:%3d:%c %s\n",
field->prev_pid,
Expand Down

0 comments on commit 4ca5308

Please sign in to comment.