Messages in this thread Patch in this message | | | Date | Thu, 6 Nov 2008 16:30:23 -0800 | From | Ken Chen <> | Subject | Re: [patch] add /proc/pid/stack to dump task's stack trace |
| |
On Thu, Nov 6, 2008 at 12:35 PM, Ingo Molnar <mingo@elte.hu> wrote: >> +static int proc_pid_stack(struct task_struct *task, char *buffer) >> +{ >> + for (i = 0; i < trace.nr_entries; i++) { >> + len += sprintf(buffer + len, "[<%p>] %pS\n", >> + (void *)entries[i], (void >> *)entries[i]); > > hm, this looks like a potential buffer overflow - isnt 'buffer' here > only valid up to the next PAGE_SIZE boundary?
Yeah. the size of buffer allocated for printing is done at upper call site in proc_info_read(). By the time we reach here, the size info is lost. It would be too much churn to add a argument to the read method of proc_op. Since these functions are all in one file, I moved PROC_BLOCK_SIZE up so it can be used to check buffer length. Would that be enough? Lots of other proc read methods don't check against buffer overrun, I suppose those should be fixed as well.
updated patch that also fixed other comments.
Signed-off-by: Ken Chen <kenchen@google.com>
index bcceb99..11f5b75 100644 --- a/Documentation/filesystems/proc.txt +++ b/Documentation/filesystems/proc.txt @@ -139,6 +139,7 @@ Table 1-1: Process specific entries in /proc statm Process memory status information status Process status in human readable form wchan If CONFIG_KALLSYMS is set, a pre-decoded wchan + stack Report full stack trace, enable via CONFIG_STACKTRACE smaps Extension based on maps, the rss size for each mapped file .............................................................................. diff --git a/fs/proc/base.c b/fs/proc/base.c index 486cf3f..8fb293d 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -65,6 +65,7 @@ #include <linux/mm.h> #include <linux/rcupdate.h> #include <linux/kallsyms.h> +#include <linux/stacktrace.h> #include <linux/resource.h> #include <linux/module.h> #include <linux/mount.h> @@ -130,6 +131,12 @@ struct pid_entry { { .proc_show = &proc_##OTYPE } ) /* + * buffer size used for proc read. See proc_info_read(). + * 4K page size but our output routines use some slack for overruns + */ +#define PROC_BLOCK_SIZE (3*1024) + +/* * Count the number of hardlinks for the pid_entry table, excluding the . * and .. links. */ @@ -340,6 +347,37 @@ static int proc_pid_wchan } #endif /* CONFIG_KALLSYMS */ +#ifdef CONFIG_STACKTRACE +#define MAX_STACK_TRACE_DEPTH 64 +static int proc_pid_stack(struct task_struct *task, char *buffer) +{ + int i, len = 0; + unsigned long *entries; + struct stack_trace trace; + + entries = kmalloc(sizeof(*entries) * MAX_STACK_TRACE_DEPTH, GFP_KERNEL); + if (!entries) + return -ENOMEM; + + trace.nr_entries = 0; + trace.max_entries = MAX_STACK_TRACE_DEPTH; + trace.entries = entries; + trace.skip = 0; + + read_lock(&tasklist_lock); + save_stack_trace_tsk(task, &trace); + read_unlock(&tasklist_lock); + + for (i = 0; i < trace.nr_entries; i++) { + len += snprintf(buffer + len, PROC_BLOCK_SIZE - len, + "[<%p>] %pS\n", + (void *)entries[i], (void *)entries[i]); + } + kfree(entries); + return len; +} +#endif + #ifdef CONFIG_SCHEDSTATS /* * Provides /proc/PID/schedstat @@ -688,8 +726,6 @@ static const struct file_operations proc_mountstats .release = mounts_release, }; -#define PROC_BLOCK_SIZE (3*1024) /* 4K page size but our output routines use some slack for overruns */ - static ssize_t proc_info_read(struct file * file, char __user * buf, size_t count, loff_t *ppos) { @@ -2491,6 +2527,9 @@ static const struct pid_entry tgid_base_stuff[] = { #ifdef CONFIG_KALLSYMS INF("wchan", S_IRUGO, pid_wchan), #endif +#ifdef CONFIG_STACKTRACE + INF("stack", S_IRUSR, pid_stack), +#endif #ifdef CONFIG_SCHEDSTATS INF("schedstat", S_IRUGO, pid_schedstat), #endif
| |