tracing: fix race condition reading saved tgids

Commit 939c7a4f04fc ("tracing: Introduce saved_cmdlines_size file")
introduced ability to change saved cmdlines size. This resized saved
command lines but missed resizing tgid mapping as well.

Another issue is that when the resize happens, it removes saved command
lines and reallocates new memory for it. This introduced a race
condition when reading the global savecmd as this can be freed in the
middle of accessing it causing a use after free access. Fix this by
implementing locking.

Bug 1954563

Signed-off-by: Adrian Salido <salidoa@google.com>
Change-Id: I334791ac35f8bcbd34362ed112aa624275a46947
Reviewed-on: https://git-master.nvidia.com/r/1560443
GVS: Gerrit_Virtual_Submit
Reviewed-by: James Huang <jamehuang@nvidia.com>
Tested-by: James Huang <jamehuang@nvidia.com>
Reviewed-by: Hayden Du <haydend@nvidia.com>
Tested-by: Hayden Du <haydend@nvidia.com>

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 587a383c413d5ef8038e79989bb801bf1c5c8542..db9881eda8301412a18fe796c9dc5fcba6546d0a 100644 (file)
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -1352,11 +1352,11 @@ void tracing_reset_all_online_cpus(void)
 
 #define SAVED_CMDLINES_DEFAULT 128
 #define NO_CMDLINE_MAP UINT_MAX
-static unsigned saved_tgids[SAVED_CMDLINES_DEFAULT];
 static arch_spinlock_t trace_cmdline_lock = __ARCH_SPIN_LOCK_UNLOCKED;
 struct saved_cmdlines_buffer {
        unsigned map_pid_to_cmdline[PID_MAX_DEFAULT+1];
        unsigned *map_cmdline_to_pid;
+       unsigned *map_cmdline_to_tgid;
        unsigned cmdline_num;
        int cmdline_idx;
        char *saved_cmdlines;
@@ -1390,12 +1390,23 @@ static int allocate_cmdlines_buffer(unsigned int val,
                return -ENOMEM;
        }
 
+       s->map_cmdline_to_tgid = kmalloc_array(val,
+                                              sizeof(*s->map_cmdline_to_tgid),
+                                              GFP_KERNEL);
+       if (!s->map_cmdline_to_tgid) {
+               kfree(s->map_cmdline_to_pid);
+               kfree(s->saved_cmdlines);
+               return -ENOMEM;
+       }
+
        s->cmdline_idx = 0;
        s->cmdline_num = val;
        memset(&s->map_pid_to_cmdline, NO_CMDLINE_MAP,
               sizeof(s->map_pid_to_cmdline));
        memset(s->map_cmdline_to_pid, NO_CMDLINE_MAP,
               val * sizeof(*s->map_cmdline_to_pid));
+       memset(s->map_cmdline_to_tgid, NO_CMDLINE_MAP,
+              val * sizeof(*s->map_cmdline_to_tgid));
 
        return 0;
 }
@@ -1561,14 +1572,17 @@ static int trace_save_cmdline(struct task_struct *tsk)
        if (!tsk->pid || unlikely(tsk->pid > PID_MAX_DEFAULT))
                return 0;
 
+       preempt_disable();
        /*
         * It's not the end of the world if we don't get
         * the lock, but we also don't want to spin
         * nor do we want to disable interrupts,
         * so if we miss here, then better luck next time.
         */
-       if (!arch_spin_trylock(&trace_cmdline_lock))
+       if (!arch_spin_trylock(&trace_cmdline_lock)) {
+               preempt_enable();
                return 0;
+       }
 
        idx = savedcmd->map_pid_to_cmdline[tsk->pid];
        if (idx == NO_CMDLINE_MAP) {
@@ -1591,8 +1605,9 @@ static int trace_save_cmdline(struct task_struct *tsk)
        }
 
        set_cmdline(idx, tsk->comm);
-       saved_tgids[idx] = tsk->tgid;
+       savedcmd->map_cmdline_to_tgid[idx] = tsk->tgid;
        arch_spin_unlock(&trace_cmdline_lock);
+       preempt_enable();
 
        return 1;
 }
@@ -1634,19 +1649,29 @@ void trace_find_cmdline(int pid, char comm[])
        preempt_enable();
 }
 
-int trace_find_tgid(int pid)
+static int __find_tgid_locked(int pid)
 {
        unsigned map;
        int tgid;
 
-       preempt_disable();
-       arch_spin_lock(&trace_cmdline_lock);
        map = savedcmd->map_pid_to_cmdline[pid];
        if (map != NO_CMDLINE_MAP)
-               tgid = saved_tgids[map];
+               tgid = savedcmd->map_cmdline_to_tgid[map];
        else
                tgid = -1;
 
+       return tgid;
+}
+
+int trace_find_tgid(int pid)
+{
+       int tgid;
+
+       preempt_disable();
+       arch_spin_lock(&trace_cmdline_lock);
+
+       tgid = __find_tgid_locked(pid);
+
        arch_spin_unlock(&trace_cmdline_lock);
        preempt_enable();
 
@@ -3961,10 +3986,15 @@ tracing_saved_cmdlines_size_read(struct file *filp, char __user *ubuf,
 {
        char buf[64];
        int r;
+       unsigned int n;
 
+       preempt_disable();
        arch_spin_lock(&trace_cmdline_lock);
-       r = scnprintf(buf, sizeof(buf), "%u\n", savedcmd->cmdline_num);
+       n = savedcmd->cmdline_num;
        arch_spin_unlock(&trace_cmdline_lock);
+       preempt_enable();
+
+       r = scnprintf(buf, sizeof(buf), "%u\n", n);
 
        return simple_read_from_buffer(ubuf, cnt, ppos, buf, r);
 }
@@ -3973,6 +4003,7 @@ static void free_saved_cmdlines_buffer(struct saved_cmdlines_buffer *s)
 {
        kfree(s->saved_cmdlines);
        kfree(s->map_cmdline_to_pid);
+       kfree(s->map_cmdline_to_tgid);
        kfree(s);
 }
 
@@ -3989,10 +4020,12 @@ static int tracing_resize_saved_cmdlines(unsigned int val)
                return -ENOMEM;
        }
 
+       preempt_disable();
        arch_spin_lock(&trace_cmdline_lock);
        savedcmd_temp = savedcmd;
        savedcmd = s;
        arch_spin_unlock(&trace_cmdline_lock);
+       preempt_enable();
        free_saved_cmdlines_buffer(savedcmd_temp);
 
        return 0;
@@ -4211,33 +4244,61 @@ tracing_saved_tgids_read(struct file *file, char __user *ubuf,
        char *file_buf;
        char *buf;
        int len = 0;
-       int pid;
        int i;
+       int *pids;
+       int n = 0;
 
-       file_buf = kmalloc(SAVED_CMDLINES_DEFAULT*(16+1+16), GFP_KERNEL);
-       if (!file_buf)
-               return -ENOMEM;
+       preempt_disable();
+       arch_spin_lock(&trace_cmdline_lock);
 
-       buf = file_buf;
+       pids = kmalloc_array(savedcmd->cmdline_num, 2*sizeof(int), GFP_KERNEL);
+       if (!pids) {
+               arch_spin_unlock(&trace_cmdline_lock);
+               preempt_enable();
+               return -ENOMEM;
+       }
 
-       for (i = 0; i < SAVED_CMDLINES_DEFAULT; i++) {
-               int tgid;
-               int r;
+       for (i = 0; i < savedcmd->cmdline_num; i++) {
+               int pid;
 
                pid = savedcmd->map_cmdline_to_pid[i];
                if (pid == -1 || pid == NO_CMDLINE_MAP)
                        continue;
 
-               tgid = trace_find_tgid(pid);
-               r = sprintf(buf, "%d %d\n", pid, tgid);
+               pids[n] = pid;
+               pids[n+1] = __find_tgid_locked(pid);
+               n += 2;
+       }
+       arch_spin_unlock(&trace_cmdline_lock);
+       preempt_enable();
+
+       if (n == 0) {
+               kfree(pids);
+               return 0;
+       }
+
+       /* enough to hold max pair of pids + space, lr and nul */
+       len = n * 12;
+       file_buf = kmalloc(len, GFP_KERNEL);
+       if (!file_buf) {
+               kfree(pids);
+               return -ENOMEM;
+       }
+
+       buf = file_buf;
+       for (i = 0; i < n && len > 0; i += 2) {
+               int r;
+
+               r = snprintf(buf, len, "%d %d\n", pids[i], pids[i+1]);
                buf += r;
-               len += r;
+               len -= r;
        }
 
        len = simple_read_from_buffer(ubuf, cnt, ppos,
-                                     file_buf, len);
+                                     file_buf, buf - file_buf);
 
        kfree(file_buf);
+       kfree(pids);
 
        return len;
 }