lkml.org 
[lkml]   [2021]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 5.12 210/242] tracing: Simplify & fix saved_tgids logic
    Date
    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
    @@ -5352,37 +5352,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)
    @@ -5391,9 +5374,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;
    }


    \
     
     \ /
      Last update: 2021-07-15 21:25    [W:2.638 / U:0.072 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site