Commit 5f639fdcd8c186c8128c616e94a7e7b159c968ae

Authored by Chris Metcalf
1 parent efb734d8ed

arch/tile: various bugs in stack backtracer

Fix a long-standing bug in the stack backtracer where we would print
garbage to the console instead of kernel function names, if the kernel
wasn't built with symbol support (e.g. mboot).

Make sure to tag every line of userspace backtrace output if we actually
have the mmap_sem, since that way if there's no tag, we know that it's
because we couldn't trylock the semaphore.

Stop doing a TLB flush and examining page tables during backtrace.
Instead, just trust that __copy_from_user_inatomic() will properly fault
and return a failure, which it should do in all cases.

Fix a latent bug where the backtracer would directly examine a signal
context in user space, rather than copying it safely to kernel memory
first.  This meant that a race with another thread could potentially
have caused a kernel panic.

Guard against unaligned sp when trying to restart backtrace at an
interrupt or signal handler point in the kernel backtracer.

Report kernel symbolic information for the call instruction rather
than for the following instruction.  We still report the actual numeric
address corresponding to the instruction after the call, for the sake
of consistency with the normal expectations for stack backtracers.

Signed-off-by: Chris Metcalf <cmetcalf@tilera.com>

Showing 2 changed files with 112 additions and 120 deletions Side-by-side Diff

arch/tile/include/asm/stack.h
... ... @@ -25,7 +25,6 @@
25 25 struct KBacktraceIterator {
26 26 BacktraceIterator it;
27 27 struct task_struct *task; /* task we are backtracing */
28   - pte_t *pgtable; /* page table for user space access */
29 28 int end; /* iteration complete. */
30 29 int new_context; /* new context is starting */
31 30 int profile; /* profiling, so stop on async intrpt */
arch/tile/kernel/stack.c
... ... @@ -21,9 +21,10 @@
21 21 #include <linux/stacktrace.h>
22 22 #include <linux/uaccess.h>
23 23 #include <linux/mmzone.h>
  24 +#include <linux/dcache.h>
  25 +#include <linux/fs.h>
24 26 #include <asm/backtrace.h>
25 27 #include <asm/page.h>
26   -#include <asm/tlbflush.h>
27 28 #include <asm/ucontext.h>
28 29 #include <asm/switch_to.h>
29 30 #include <asm/sigframe.h>
30 31  
31 32  
... ... @@ -45,72 +46,23 @@
45 46 return sp >= kstack_base && sp < kstack_base + THREAD_SIZE;
46 47 }
47 48  
48   -/* Is address valid for reading? */
49   -static int valid_address(struct KBacktraceIterator *kbt, unsigned long address)
50   -{
51   - HV_PTE *l1_pgtable = kbt->pgtable;
52   - HV_PTE *l2_pgtable;
53   - unsigned long pfn;
54   - HV_PTE pte;
55   - struct page *page;
56   -
57   - if (l1_pgtable == NULL)
58   - return 0; /* can't read user space in other tasks */
59   -
60   -#ifdef CONFIG_64BIT
61   - /* Find the real l1_pgtable by looking in the l0_pgtable. */
62   - pte = l1_pgtable[HV_L0_INDEX(address)];
63   - if (!hv_pte_get_present(pte))
64   - return 0;
65   - pfn = hv_pte_get_pfn(pte);
66   - if (pte_huge(pte)) {
67   - if (!pfn_valid(pfn)) {
68   - pr_err("L0 huge page has bad pfn %#lx\n", pfn);
69   - return 0;
70   - }
71   - return hv_pte_get_present(pte) && hv_pte_get_readable(pte);
72   - }
73   - page = pfn_to_page(pfn);
74   - BUG_ON(PageHighMem(page)); /* No HIGHMEM on 64-bit. */
75   - l1_pgtable = (HV_PTE *)pfn_to_kaddr(pfn);
76   -#endif
77   - pte = l1_pgtable[HV_L1_INDEX(address)];
78   - if (!hv_pte_get_present(pte))
79   - return 0;
80   - pfn = hv_pte_get_pfn(pte);
81   - if (pte_huge(pte)) {
82   - if (!pfn_valid(pfn)) {
83   - pr_err("huge page has bad pfn %#lx\n", pfn);
84   - return 0;
85   - }
86   - return hv_pte_get_present(pte) && hv_pte_get_readable(pte);
87   - }
88   -
89   - page = pfn_to_page(pfn);
90   - if (PageHighMem(page)) {
91   - pr_err("L2 page table not in LOWMEM (%#llx)\n",
92   - HV_PFN_TO_CPA(pfn));
93   - return 0;
94   - }
95   - l2_pgtable = (HV_PTE *)pfn_to_kaddr(pfn);
96   - pte = l2_pgtable[HV_L2_INDEX(address)];
97   - return hv_pte_get_present(pte) && hv_pte_get_readable(pte);
98   -}
99   -
100 49 /* Callback for backtracer; basically a glorified memcpy */
101 50 static bool read_memory_func(void *result, unsigned long address,
102 51 unsigned int size, void *vkbt)
103 52 {
104 53 int retval;
105 54 struct KBacktraceIterator *kbt = (struct KBacktraceIterator *)vkbt;
  55 +
  56 + if (address == 0)
  57 + return 0;
106 58 if (__kernel_text_address(address)) {
107 59 /* OK to read kernel code. */
108 60 } else if (address >= PAGE_OFFSET) {
109 61 /* We only tolerate kernel-space reads of this task's stack */
110 62 if (!in_kernel_stack(kbt, address))
111 63 return 0;
112   - } else if (!valid_address(kbt, address)) {
113   - return 0; /* invalid user-space address */
  64 + } else if (!kbt->is_current) {
  65 + return 0; /* can't read from other user address spaces */
114 66 }
115 67 pagefault_disable();
116 68 retval = __copy_from_user_inatomic(result,
... ... @@ -128,6 +80,8 @@
128 80 unsigned long sp = kbt->it.sp;
129 81 struct pt_regs *p;
130 82  
  83 + if (sp % sizeof(long) != 0)
  84 + return NULL;
131 85 if (!in_kernel_stack(kbt, sp))
132 86 return NULL;
133 87 if (!in_kernel_stack(kbt, sp + C_ABI_SAVE_AREA_SIZE + PTREGS_SIZE-1))
134 88  
135 89  
136 90  
137 91  
... ... @@ -170,27 +124,27 @@
170 124 }
171 125  
172 126 /* Return a pt_regs pointer for a valid signal handler frame */
173   -static struct pt_regs *valid_sigframe(struct KBacktraceIterator* kbt)
  127 +static struct pt_regs *valid_sigframe(struct KBacktraceIterator* kbt,
  128 + struct rt_sigframe* kframe)
174 129 {
175 130 BacktraceIterator *b = &kbt->it;
176 131  
177   - if (b->pc == VDSO_BASE) {
178   - struct rt_sigframe *frame;
179   - unsigned long sigframe_top =
180   - b->sp + sizeof(struct rt_sigframe) - 1;
181   - if (!valid_address(kbt, b->sp) ||
182   - !valid_address(kbt, sigframe_top)) {
183   - if (kbt->verbose)
184   - pr_err(" (odd signal: sp %#lx?)\n",
185   - (unsigned long)(b->sp));
  132 + if (b->pc == VDSO_BASE && b->sp < PAGE_OFFSET &&
  133 + b->sp % sizeof(long) == 0) {
  134 + int retval;
  135 + pagefault_disable();
  136 + retval = __copy_from_user_inatomic(
  137 + kframe, (void __user __force *)b->sp,
  138 + sizeof(*kframe));
  139 + pagefault_enable();
  140 + if (retval != 0 ||
  141 + (unsigned int)(kframe->info.si_signo) >= _NSIG)
186 142 return NULL;
187   - }
188   - frame = (struct rt_sigframe *)b->sp;
189 143 if (kbt->verbose) {
190 144 pr_err(" <received signal %d>\n",
191   - frame->info.si_signo);
  145 + kframe->info.si_signo);
192 146 }
193   - return (struct pt_regs *)&frame->uc.uc_mcontext;
  147 + return (struct pt_regs *)&kframe->uc.uc_mcontext;
194 148 }
195 149 return NULL;
196 150 }
197 151  
... ... @@ -203,10 +157,11 @@
203 157 static int KBacktraceIterator_restart(struct KBacktraceIterator *kbt)
204 158 {
205 159 struct pt_regs *p;
  160 + struct rt_sigframe kframe;
206 161  
207 162 p = valid_fault_handler(kbt);
208 163 if (p == NULL)
209   - p = valid_sigframe(kbt);
  164 + p = valid_sigframe(kbt, &kframe);
210 165 if (p == NULL)
211 166 return 0;
212 167 backtrace_init(&kbt->it, read_memory_func, kbt,
213 168  
214 169  
215 170  
216 171  
... ... @@ -266,41 +221,19 @@
266 221  
267 222 /*
268 223 * Set up callback information. We grab the kernel stack base
269   - * so we will allow reads of that address range, and if we're
270   - * asking about the current process we grab the page table
271   - * so we can check user accesses before trying to read them.
272   - * We flush the TLB to avoid any weird skew issues.
  224 + * so we will allow reads of that address range.
273 225 */
274   - is_current = (t == NULL);
  226 + is_current = (t == NULL || t == current);
275 227 kbt->is_current = is_current;
276 228 if (is_current)
277 229 t = validate_current();
278 230 kbt->task = t;
279   - kbt->pgtable = NULL;
280 231 kbt->verbose = 0; /* override in caller if desired */
281 232 kbt->profile = 0; /* override in caller if desired */
282 233 kbt->end = KBT_ONGOING;
283   - kbt->new_context = 0;
284   - if (is_current) {
285   - HV_PhysAddr pgdir_pa = hv_inquire_context().page_table;
286   - if (pgdir_pa == (unsigned long)swapper_pg_dir - PAGE_OFFSET) {
287   - /*
288   - * Not just an optimization: this also allows
289   - * this to work at all before va/pa mappings
290   - * are set up.
291   - */
292   - kbt->pgtable = swapper_pg_dir;
293   - } else {
294   - struct page *page = pfn_to_page(PFN_DOWN(pgdir_pa));
295   - if (!PageHighMem(page))
296   - kbt->pgtable = __va(pgdir_pa);
297   - else
298   - pr_err("page table not in LOWMEM"
299   - " (%#llx)\n", pgdir_pa);
300   - }
301   - local_flush_tlb_all();
  234 + kbt->new_context = 1;
  235 + if (is_current)
302 236 validate_stack(regs);
303   - }
304 237  
305 238 if (regs == NULL) {
306 239 if (is_current || t->state == TASK_RUNNING) {
... ... @@ -346,6 +279,78 @@
346 279 }
347 280 EXPORT_SYMBOL(KBacktraceIterator_next);
348 281  
  282 +static void describe_addr(struct KBacktraceIterator *kbt,
  283 + unsigned long address,
  284 + int have_mmap_sem, char *buf, size_t bufsize)
  285 +{
  286 + struct vm_area_struct *vma;
  287 + size_t namelen, remaining;
  288 + unsigned long size, offset, adjust;
  289 + char *p, *modname;
  290 + const char *name;
  291 + int rc;
  292 +
  293 + /*
  294 + * Look one byte back for every caller frame (i.e. those that
  295 + * aren't a new context) so we look up symbol data for the
  296 + * call itself, not the following instruction, which may be on
  297 + * a different line (or in a different function).
  298 + */
  299 + adjust = !kbt->new_context;
  300 + address -= adjust;
  301 +
  302 + if (address >= PAGE_OFFSET) {
  303 + /* Handle kernel symbols. */
  304 + BUG_ON(bufsize < KSYM_NAME_LEN);
  305 + name = kallsyms_lookup(address, &size, &offset,
  306 + &modname, buf);
  307 + if (name == NULL) {
  308 + buf[0] = '\0';
  309 + return;
  310 + }
  311 + namelen = strlen(buf);
  312 + remaining = (bufsize - 1) - namelen;
  313 + p = buf + namelen;
  314 + rc = snprintf(p, remaining, "+%#lx/%#lx ",
  315 + offset + adjust, size);
  316 + if (modname && rc < remaining)
  317 + snprintf(p + rc, remaining - rc, "[%s] ", modname);
  318 + buf[bufsize-1] = '\0';
  319 + return;
  320 + }
  321 +
  322 + /* If we don't have the mmap_sem, we can't show any more info. */
  323 + buf[0] = '\0';
  324 + if (!have_mmap_sem)
  325 + return;
  326 +
  327 + /* Find vma info. */
  328 + vma = find_vma(kbt->task->mm, address);
  329 + if (vma == NULL || address < vma->vm_start) {
  330 + snprintf(buf, bufsize, "[unmapped address] ");
  331 + return;
  332 + }
  333 +
  334 + if (vma->vm_file) {
  335 + char *s;
  336 + p = d_path(&vma->vm_file->f_path, buf, bufsize);
  337 + if (IS_ERR(p))
  338 + p = "?";
  339 + s = strrchr(p, '/');
  340 + if (s)
  341 + p = s+1;
  342 + } else {
  343 + p = "anon";
  344 + }
  345 +
  346 + /* Generate a string description of the vma info. */
  347 + namelen = strlen(p);
  348 + remaining = (bufsize - 1) - namelen;
  349 + memmove(buf, p, namelen);
  350 + snprintf(buf + namelen, remaining, "[%lx+%lx] ",
  351 + vma->vm_start, vma->vm_end - vma->vm_start);
  352 +}
  353 +
349 354 /*
350 355 * This method wraps the backtracer's more generic support.
351 356 * It is only invoked from the architecture-specific code; show_stack()
... ... @@ -354,6 +359,7 @@
354 359 void tile_show_stack(struct KBacktraceIterator *kbt, int headers)
355 360 {
356 361 int i;
  362 + int have_mmap_sem = 0;
357 363  
358 364 if (headers) {
359 365 /*
360 366  
361 367  
362 368  
... ... @@ -370,31 +376,16 @@
370 376 kbt->verbose = 1;
371 377 i = 0;
372 378 for (; !KBacktraceIterator_end(kbt); KBacktraceIterator_next(kbt)) {
373   - char *modname;
374   - const char *name;
375   - unsigned long address = kbt->it.pc;
376   - unsigned long offset, size;
377 379 char namebuf[KSYM_NAME_LEN+100];
  380 + unsigned long address = kbt->it.pc;
378 381  
379   - if (address >= PAGE_OFFSET)
380   - name = kallsyms_lookup(address, &size, &offset,
381   - &modname, namebuf);
382   - else
383   - name = NULL;
  382 + /* Try to acquire the mmap_sem as we pass into userspace. */
  383 + if (address < PAGE_OFFSET && !have_mmap_sem && kbt->task->mm)
  384 + have_mmap_sem =
  385 + down_read_trylock(&kbt->task->mm->mmap_sem);
384 386  
385   - if (!name)
386   - namebuf[0] = '\0';
387   - else {
388   - size_t namelen = strlen(namebuf);
389   - size_t remaining = (sizeof(namebuf) - 1) - namelen;
390   - char *p = namebuf + namelen;
391   - int rc = snprintf(p, remaining, "+%#lx/%#lx ",
392   - offset, size);
393   - if (modname && rc < remaining)
394   - snprintf(p + rc, remaining - rc,
395   - "[%s] ", modname);
396   - namebuf[sizeof(namebuf)-1] = '\0';
397   - }
  387 + describe_addr(kbt, address, have_mmap_sem,
  388 + namebuf, sizeof(namebuf));
398 389  
399 390 pr_err(" frame %d: 0x%lx %s(sp 0x%lx)\n",
400 391 i++, address, namebuf, (unsigned long)(kbt->it.sp));
... ... @@ -409,6 +400,8 @@
409 400 pr_err("Stack dump stopped; next frame identical to this one\n");
410 401 if (headers)
411 402 pr_err("Stack dump complete\n");
  403 + if (have_mmap_sem)
  404 + up_read(&kbt->task->mm->mmap_sem);
412 405 }
413 406 EXPORT_SYMBOL(tile_show_stack);
414 407