From 3753ea415f57b8e2aa6b7a6b4596ec68f986ce09 Mon Sep 17 00:00:00 2001 From: Kris Van Hees Date: Fri, 4 Apr 2014 10:04:58 -0400 Subject: [PATCH] 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 Reviewed-by: Chuck Anderson --- fs/exec.c | 2 +- include/linux/dtrace_psinfo.h | 21 ++++++++++-- kernel/dtrace/dtrace_os.c | 61 +++++++++++++++++++++++++++-------- kernel/fork.c | 6 ++-- 4 files changed, 71 insertions(+), 19 deletions(-) diff --git a/fs/exec.c b/fs/exec.c index a30cdfb0e7c0..b0c9033613a0 100644 --- 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 8336aeb8918e..050d91f1d9b4 100644 --- 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 a15d36a2a9d9..18f03082c5b7 100644 --- 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 22933f7fa296..a5431f06b398 100644 --- 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 -- 2.50.1