| From b81b3e959adb107cd5b36c7dc5ba1364bbd31eb2 Mon Sep 17 00:00:00 2001 |
| From: Paul Burton <paulburton@google.com> |
| Date: Tue, 29 Jun 2021 17:34:05 -0700 |
| Subject: tracing: Simplify & fix saved_tgids logic |
| |
| From: Paul Burton <paulburton@google.com> |
| |
| commit b81b3e959adb107cd5b36c7dc5ba1364bbd31eb2 upstream. |
| |
| The tgid_map array records a mapping from pid to tgid, where the index |
| of an entry within the array is the pid & the value stored at that index |
| is the tgid. |
| |
| The saved_tgids_next() function iterates over pointers into the tgid_map |
| array & dereferences the pointers which results in the tgid, but then it |
| passes that dereferenced value to trace_find_tgid() which treats it as a |
| pid & does a further lookup within the tgid_map array. It seems likely |
| that the intent here was to skip over entries in tgid_map for which the |
| recorded tgid is zero, but instead we end up skipping over entries for |
| which the thread group leader hasn't yet had its own tgid recorded in |
| tgid_map. |
| |
| A minimal fix would be to remove the call to trace_find_tgid, turning: |
| |
| if (trace_find_tgid(*ptr)) |
| |
| into: |
| |
| if (*ptr) |
| |
| ..but it seems like this logic can be much simpler if we simply let |
| seq_read() iterate over the whole tgid_map array & filter out empty |
| entries by returning SEQ_SKIP from saved_tgids_show(). Here we take that |
| approach, removing the incorrect logic here entirely. |
| |
| Link: https://lkml.kernel.org/r/20210630003406.4013668-1-paulburton@google.com |
| |
| Fixes: d914ba37d714 ("tracing: Add support for recording tgid of tasks") |
| Cc: Ingo Molnar <mingo@redhat.com> |
| Cc: Joel Fernandes <joelaf@google.com> |
| Cc: <stable@vger.kernel.org> |
| Signed-off-by: Paul Burton <paulburton@google.com> |
| Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| kernel/trace/trace.c | 38 +++++++++++++------------------------- |
| 1 file changed, 13 insertions(+), 25 deletions(-) |
| |
| --- a/kernel/trace/trace.c |
| +++ b/kernel/trace/trace.c |
| @@ -5284,37 +5284,20 @@ static const struct file_operations trac |
| |
| static void *saved_tgids_next(struct seq_file *m, void *v, loff_t *pos) |
| { |
| - int *ptr = v; |
| + int pid = ++(*pos); |
| |
| - if (*pos || m->count) |
| - ptr++; |
| - |
| - (*pos)++; |
| - |
| - for (; ptr <= &tgid_map[PID_MAX_DEFAULT]; ptr++) { |
| - if (trace_find_tgid(*ptr)) |
| - return ptr; |
| - } |
| + if (pid > PID_MAX_DEFAULT) |
| + return NULL; |
| |
| - return NULL; |
| + return &tgid_map[pid]; |
| } |
| |
| static void *saved_tgids_start(struct seq_file *m, loff_t *pos) |
| { |
| - void *v; |
| - loff_t l = 0; |
| - |
| - if (!tgid_map) |
| + if (!tgid_map || *pos > PID_MAX_DEFAULT) |
| return NULL; |
| |
| - v = &tgid_map[0]; |
| - while (l <= *pos) { |
| - v = saved_tgids_next(m, v, &l); |
| - if (!v) |
| - return NULL; |
| - } |
| - |
| - return v; |
| + return &tgid_map[*pos]; |
| } |
| |
| static void saved_tgids_stop(struct seq_file *m, void *v) |
| @@ -5323,9 +5306,14 @@ static void saved_tgids_stop(struct seq_ |
| |
| static int saved_tgids_show(struct seq_file *m, void *v) |
| { |
| - int pid = (int *)v - tgid_map; |
| + int *entry = (int *)v; |
| + int pid = entry - tgid_map; |
| + int tgid = *entry; |
| + |
| + if (tgid == 0) |
| + return SEQ_SKIP; |
| |
| - seq_printf(m, "%d %d\n", pid, trace_find_tgid(pid)); |
| + seq_printf(m, "%d %d\n", pid, tgid); |
| return 0; |
| } |
| |