dtrace: fix leaking psinfo objects

The psinfo objects created from a kmem cache (slab) to hold information
about a task's environment and arguments can leak when a task executes
two consecutive execve() calls.  The implementation also made it possible
for tasks to have no psinfo (NULL) after a fork() unless an execve() was
done shortly after.  This commit resolved both problems by adding a refc
to the psinfo objects (so one can be shared across task parentage
relations due to fork()), and by ensuring that upon execve() any existing
psinfo gets its refc decremented and a new psinfo object gets installed
to reflect the new execution environment.

This commit also adds the necessary initialization of a psinfo object for
tasks that do not have a mm object associated with them.

Orabug: 18383027

Signed-off-by: Kris Van Hees <kris.van.hees@oracle.com>
Reviewed-by: Chuck Anderson <chuck.anderson@oracle.com>

diff --git a/fs/exec.c b/fs/exec.c
index a30cdfb0e7c0c624c94de091f3fc01e5d797e617..b0c9033613a050082796b75cae9a0e98e3c05003 100644 (file)
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1592,7 +1592,7 @@ static int do_execveat_common(int fd, struct filename *filename,
 #ifdef CONFIG_DTRACE
        dtrace_task_cleanup(current);   /* get rid of probes from old ... */
        dtrace_task_reinit(current);    /* ... be ready for probes from new */
-       current->dtrace_psinfo = dtrace_psinfo_alloc(current);
+       dtrace_psinfo_alloc(current);   /* install new psinfo object */
 #endif
        current->fs->in_exec = 0;
        current->in_execve = 0;

diff --git a/include/linux/dtrace_psinfo.h b/include/linux/dtrace_psinfo.h
index 8336aeb8918eb05392d2ea3b5408342d47157f89..050d91f1d9b407a263d395512e6c985b93d19313 100644 (file)
--- a/include/linux/dtrace_psinfo.h
+++ b/include/linux/dtrace_psinfo.h
@@ -8,6 +8,10 @@
 #define PR_ENVP_SZ             512
 
 typedef struct dtrace_psinfo {
+/* Orabug 18383027 - Remove the conditionals at the next major UEK release. */
+#ifndef __GENKSYMS__
+       atomic_t usage;
+#endif
        union {
                unsigned long argc;
                struct dtrace_psinfo *next;
@@ -17,7 +21,20 @@ typedef struct dtrace_psinfo {
        char psargs[PR_PSARGS_SZ];
 } dtrace_psinfo_t;
 
-extern dtrace_psinfo_t *dtrace_psinfo_alloc(struct task_struct *);
-extern void dtrace_psinfo_free(dtrace_psinfo_t *);
+extern void dtrace_psinfo_alloc(struct task_struct *);
+extern void dtrace_psinfo_free(struct task_struct *);
+
+#define get_psinfo(tsk)                                                              \
+       do {                                                                  \
+               if (likely((tsk)->dtrace_psinfo))                             \
+                       atomic_inc(&(tsk)->dtrace_psinfo->usage);             \
+       } while (0)
+#define put_psinfo(tsk)                                                              \
+       do {                                                                  \
+               if (likely((tsk)->dtrace_psinfo)) {                           \
+                       if (atomic_dec_and_test(&(tsk)->dtrace_psinfo->usage))\
+                               dtrace_psinfo_free(tsk);                      \
+               }                                                             \
+       } while (0)
 
 #endif /* _LINUX_DTRACE_PSINFO_H_ */

diff --git a/kernel/dtrace/dtrace_os.c b/kernel/dtrace/dtrace_os.c
index a15d36a2a9d927683048068376994f77ef35edf1..18f03082c5b78f5ecf0b8c256708a267e4c39c19 100644 (file)
--- a/kernel/dtrace/dtrace_os.c
+++ b/kernel/dtrace/dtrace_os.c
@@ -76,16 +76,21 @@ EXPORT_SYMBOL(dtrace_os_exit);
 /*
  * Allocate a new dtrace_psinfo_t structure.
  */
-dtrace_psinfo_t *dtrace_psinfo_alloc(struct task_struct *task)
+void dtrace_psinfo_alloc(struct task_struct *tsk)
 {
        dtrace_psinfo_t         *psinfo;
        struct mm_struct        *mm;
 
+       if (likely(tsk->dtrace_psinfo)) {
+               put_psinfo(tsk);
+               tsk->dtrace_psinfo = NULL;      /* while we build one */
+       }
+
        psinfo = kmem_cache_alloc(psinfo_cachep, GFP_KERNEL);
        if (psinfo == NULL)
                goto fail;
 
-       mm = get_task_mm(task);
+       mm = get_task_mm(tsk);
        if (mm) {
                size_t  len = mm->arg_end - mm->arg_start;
                int     i, envc = 0;
@@ -97,13 +102,11 @@ dtrace_psinfo_t *dtrace_psinfo_alloc(struct task_struct *task)
                if (len >= PR_PSARGS_SZ)
                        len = PR_PSARGS_SZ - 1;
 
-               i = access_process_vm(task, mm->arg_start, psinfo->psargs,
+               i = access_process_vm(tsk, mm->arg_start, psinfo->psargs,
                                        len, 0);
                while (i < PR_PSARGS_SZ)
                        psinfo->psargs[i++] = 0;
 
-               mmput(mm);
-
                for (i = 0; i < len; i++) {
                        if (psinfo->psargs[i] == '\0')
                                psinfo->psargs[i] = ' ';
@@ -129,7 +132,8 @@ dtrace_psinfo_t *dtrace_psinfo_alloc(struct task_struct *task)
                if ((len = psinfo->argc) >= PR_ARGV_SZ)
                        len = PR_ARGV_SZ - 1;
 
-               psinfo->argv = kmalloc((len + 1) * sizeof(char *), GFP_KERNEL);
+               psinfo->argv = kmalloc((len + 1) * sizeof(char *),
+                                        GFP_KERNEL);
                if (psinfo->argv == NULL)
                        goto fail;
 
@@ -161,7 +165,8 @@ dtrace_psinfo_t *dtrace_psinfo_alloc(struct task_struct *task)
                if ((len = envc) >= PR_ENVP_SZ)
                        len = PR_ENVP_SZ - 1;
 
-               psinfo->envp = kmalloc((len + 1) * sizeof(char *), GFP_KERNEL);
+               psinfo->envp = kmalloc((len + 1) * sizeof(char *),
+                                        GFP_KERNEL);
                if (psinfo->envp == NULL)
                        goto fail;
 
@@ -173,12 +178,39 @@ dtrace_psinfo_t *dtrace_psinfo_alloc(struct task_struct *task)
                        p += strnlen(p, MAX_ARG_STRLEN) + 1;
                }
                psinfo->envp[len] = NULL;
+
+               mmput(mm);
+       } else {
+               size_t  len = min(TASK_COMM_LEN, PR_PSARGS_SZ);
+               int     i;
+
+               /*
+                * We end up here for tasks that do not have managed memory at
+                * all, which generally means that this is a kernel thread.
+                * If it is not, this is still safe because we know that tasks
+                * always have the comm member populated with something (even
+                * if it would be an empty string).
+                */
+               memcpy(psinfo->psargs, tsk->comm, len);
+               for (i = len; i < PR_PSARGS_SZ; i++)
+                       psinfo->psargs[i] = 0;
+
+               psinfo->argc = 0;
+               psinfo->argv = kmalloc(sizeof(char *), GFP_KERNEL);
+               psinfo->argv[0] = NULL;
+               psinfo->envp = kmalloc(sizeof(char *), GFP_KERNEL);
+               psinfo->envp[0] = NULL;
        }
 
-       return psinfo;
+       atomic_set(&psinfo->usage, 1);
+       tsk->dtrace_psinfo = psinfo;            /* new one */
+
+       return;
 
 fail:
-       pr_warning("%s: cannot allocate DTrace psinfo structure\n", __func__);
+       if (mm)
+               mmput(mm);
+
        if (psinfo) {
                if (psinfo->argv)
                        kfree(psinfo->argv);
@@ -187,8 +219,6 @@ fail:
 
                kmem_cache_free(psinfo_cachep, psinfo);
        }
-
-       return NULL;
 }
 
 static DEFINE_SPINLOCK(psinfo_lock);
@@ -232,16 +262,19 @@ static DECLARE_WORK(psinfo_cleanup, psinfo_cleaner);
 /*
  * Schedule a psinfo structure for free'ing.
  */
-void dtrace_psinfo_free(dtrace_psinfo_t *psinfo)
+void dtrace_psinfo_free(struct task_struct *tsk)
 {
        unsigned long   flags;
+       dtrace_psinfo_t *psinfo = tsk->dtrace_psinfo;
 
        /*
-        * For most tasks, we never populate any DTrace psinfo.
+        * There are (very few) tasks without psinfo...
         */
-       if (!psinfo)
+       if (unlikely(psinfo == NULL))
                return;
 
+       tsk->dtrace_psinfo = NULL;
+
        spin_lock_irqsave(&psinfo_lock, flags);
        psinfo->next = psinfo_free_list;
        psinfo_free_list = psinfo;

diff --git a/kernel/fork.c b/kernel/fork.c
index 22933f7fa29631140bdff35dd1f528aec7fb9c22..a5431f06b398a33f1ca546b0f9df02da3cabae1b 100644 (file)
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -224,7 +224,7 @@ static void account_kernel_stack(struct thread_info *ti, int account)
 void free_task(struct task_struct *tsk)
 {
 #ifdef CONFIG_DTRACE
-       dtrace_psinfo_free(tsk->dtrace_psinfo);
+       put_psinfo(tsk);
 #endif
 
        account_kernel_stack(tsk->stack, -1);
@@ -384,7 +384,9 @@ static struct task_struct *dup_task_struct(struct task_struct *orig)
        account_kernel_stack(ti, 1);
 
 #ifdef CONFIG_DTRACE
-       tsk->dtrace_psinfo = NULL;
+       if (likely(tsk->dtrace_psinfo))
+               get_psinfo(tsk);
+
        dtrace_task_init(tsk);
 #endif