Commit c34d1b4d authored by Hugh Dickins's avatar Hugh Dickins Committed by Linus Torvalds

[PATCH] mm: kill check_user_page_readable

check_user_page_readable is a problematic variant of follow_page.  It's used
only by oprofile's i386 and arm backtrace code, at interrupt time, to
establish whether a userspace stackframe is currently readable.

This is problematic, because we want to push the page_table_lock down inside
follow_page, and later split it; whereas oprofile is doing a spin_trylock on
it (in the i386 case, forgotten in the arm case), and needs that to pin
perhaps two pages spanned by the stackframe (which might be covered by
different locks when we split).

I think oprofile is going about this in the wrong way: it doesn't need to know
the area is readable (neither i386 nor arm uses read protection of user
pages), it doesn't need to pin the memory, it should simply
__copy_from_user_inatomic, and see if that succeeds or not.  Sorry, but I've
not got around to devising the sparse __user annotations for this.

Then we can eliminate check_user_page_readable, and return to a single
follow_page without the __follow_page variants.
Signed-off-by: default avatarHugh Dickins <hugh@veritas.com>
Signed-off-by: default avatarAndrew Morton <akpm@osdl.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent c0718806
...@@ -49,42 +49,22 @@ static struct frame_tail* kernel_backtrace(struct frame_tail *tail) ...@@ -49,42 +49,22 @@ static struct frame_tail* kernel_backtrace(struct frame_tail *tail)
static struct frame_tail* user_backtrace(struct frame_tail *tail) static struct frame_tail* user_backtrace(struct frame_tail *tail)
{ {
struct frame_tail buftail; struct frame_tail buftail[2];
/* hardware pte might not be valid due to dirty/accessed bit emulation /* Also check accessibility of one struct frame_tail beyond */
* so we use copy_from_user and benefit from exception fixups */ if (!access_ok(VERIFY_READ, tail, sizeof(buftail)))
if (copy_from_user(&buftail, tail, sizeof(struct frame_tail))) return NULL;
if (__copy_from_user_inatomic(buftail, tail, sizeof(buftail)))
return NULL; return NULL;
oprofile_add_trace(buftail.lr); oprofile_add_trace(buftail[0].lr);
/* frame pointers should strictly progress back up the stack /* frame pointers should strictly progress back up the stack
* (towards higher addresses) */ * (towards higher addresses) */
if (tail >= buftail.fp) if (tail >= buftail[0].fp)
return NULL; return NULL;
return buftail.fp-1; return buftail[0].fp-1;
}
/* Compare two addresses and see if they're on the same page */
#define CMP_ADDR_EQUAL(x,y,offset) ((((unsigned long) x) >> PAGE_SHIFT) \
== ((((unsigned long) y) + offset) >> PAGE_SHIFT))
/* check that the page(s) containing the frame tail are present */
static int pages_present(struct frame_tail *tail)
{
struct mm_struct * mm = current->mm;
if (!check_user_page_readable(mm, (unsigned long)tail))
return 0;
if (CMP_ADDR_EQUAL(tail, tail, 8))
return 1;
if (!check_user_page_readable(mm, ((unsigned long)tail) + 8))
return 0;
return 1;
} }
/* /*
...@@ -118,7 +98,6 @@ static int valid_kernel_stack(struct frame_tail *tail, struct pt_regs *regs) ...@@ -118,7 +98,6 @@ static int valid_kernel_stack(struct frame_tail *tail, struct pt_regs *regs)
void arm_backtrace(struct pt_regs * const regs, unsigned int depth) void arm_backtrace(struct pt_regs * const regs, unsigned int depth)
{ {
struct frame_tail *tail; struct frame_tail *tail;
unsigned long last_address = 0;
tail = ((struct frame_tail *) regs->ARM_fp) - 1; tail = ((struct frame_tail *) regs->ARM_fp) - 1;
...@@ -132,13 +111,6 @@ void arm_backtrace(struct pt_regs * const regs, unsigned int depth) ...@@ -132,13 +111,6 @@ void arm_backtrace(struct pt_regs * const regs, unsigned int depth)
return; return;
} }
while (depth-- && tail && !((unsigned long) tail & 3)) { while (depth-- && tail && !((unsigned long) tail & 3))
if ((!CMP_ADDR_EQUAL(last_address, tail, 0)
|| !CMP_ADDR_EQUAL(last_address, tail, 8))
&& !pages_present(tail))
return;
last_address = (unsigned long) tail;
tail = user_backtrace(tail); tail = user_backtrace(tail);
}
} }
...@@ -12,6 +12,7 @@ ...@@ -12,6 +12,7 @@
#include <linux/sched.h> #include <linux/sched.h>
#include <linux/mm.h> #include <linux/mm.h>
#include <asm/ptrace.h> #include <asm/ptrace.h>
#include <asm/uaccess.h>
struct frame_head { struct frame_head {
struct frame_head * ebp; struct frame_head * ebp;
...@@ -21,26 +22,22 @@ struct frame_head { ...@@ -21,26 +22,22 @@ struct frame_head {
static struct frame_head * static struct frame_head *
dump_backtrace(struct frame_head * head) dump_backtrace(struct frame_head * head)
{ {
oprofile_add_trace(head->ret); struct frame_head bufhead[2];
/* frame pointers should strictly progress back up the stack /* Also check accessibility of one struct frame_head beyond */
* (towards higher addresses) */ if (!access_ok(VERIFY_READ, head, sizeof(bufhead)))
if (head >= head->ebp) return NULL;
if (__copy_from_user_inatomic(bufhead, head, sizeof(bufhead)))
return NULL; return NULL;
return head->ebp; oprofile_add_trace(bufhead[0].ret);
}
/* check that the page(s) containing the frame head are present */
static int pages_present(struct frame_head * head)
{
struct mm_struct * mm = current->mm;
/* FIXME: only necessary once per page */ /* frame pointers should strictly progress back up the stack
if (!check_user_page_readable(mm, (unsigned long)head)) * (towards higher addresses) */
return 0; if (head >= bufhead[0].ebp)
return NULL;
return check_user_page_readable(mm, (unsigned long)(head + 1)); return bufhead[0].ebp;
} }
/* /*
...@@ -97,15 +94,6 @@ x86_backtrace(struct pt_regs * const regs, unsigned int depth) ...@@ -97,15 +94,6 @@ x86_backtrace(struct pt_regs * const regs, unsigned int depth)
return; return;
} }
#ifdef CONFIG_SMP while (depth-- && head)
if (!spin_trylock(&current->mm->page_table_lock))
return;
#endif
while (depth-- && head && pages_present(head))
head = dump_backtrace(head); head = dump_backtrace(head);
#ifdef CONFIG_SMP
spin_unlock(&current->mm->page_table_lock);
#endif
} }
...@@ -944,7 +944,6 @@ extern struct page * vmalloc_to_page(void *addr); ...@@ -944,7 +944,6 @@ extern struct page * vmalloc_to_page(void *addr);
extern unsigned long vmalloc_to_pfn(void *addr); extern unsigned long vmalloc_to_pfn(void *addr);
extern struct page * follow_page(struct mm_struct *mm, unsigned long address, extern struct page * follow_page(struct mm_struct *mm, unsigned long address,
int write); int write);
extern int check_user_page_readable(struct mm_struct *mm, unsigned long address);
int remap_pfn_range(struct vm_area_struct *, unsigned long, int remap_pfn_range(struct vm_area_struct *, unsigned long,
unsigned long, unsigned long, pgprot_t); unsigned long, unsigned long, pgprot_t);
......
...@@ -809,8 +809,7 @@ unsigned long zap_page_range(struct vm_area_struct *vma, unsigned long address, ...@@ -809,8 +809,7 @@ unsigned long zap_page_range(struct vm_area_struct *vma, unsigned long address,
* Do a quick page-table lookup for a single page. * Do a quick page-table lookup for a single page.
* mm->page_table_lock must be held. * mm->page_table_lock must be held.
*/ */
static struct page *__follow_page(struct mm_struct *mm, unsigned long address, struct page *follow_page(struct mm_struct *mm, unsigned long address, int write)
int read, int write, int accessed)
{ {
pgd_t *pgd; pgd_t *pgd;
pud_t *pud; pud_t *pud;
...@@ -846,16 +845,12 @@ static struct page *__follow_page(struct mm_struct *mm, unsigned long address, ...@@ -846,16 +845,12 @@ static struct page *__follow_page(struct mm_struct *mm, unsigned long address,
if (pte_present(pte)) { if (pte_present(pte)) {
if (write && !pte_write(pte)) if (write && !pte_write(pte))
goto out; goto out;
if (read && !pte_read(pte))
goto out;
pfn = pte_pfn(pte); pfn = pte_pfn(pte);
if (pfn_valid(pfn)) { if (pfn_valid(pfn)) {
page = pfn_to_page(pfn); page = pfn_to_page(pfn);
if (accessed) {
if (write && !pte_dirty(pte) &&!PageDirty(page)) if (write && !pte_dirty(pte) &&!PageDirty(page))
set_page_dirty(page); set_page_dirty(page);
mark_page_accessed(page); mark_page_accessed(page);
}
return page; return page;
} }
} }
...@@ -864,22 +859,6 @@ out: ...@@ -864,22 +859,6 @@ out:
return NULL; return NULL;
} }
inline struct page *
follow_page(struct mm_struct *mm, unsigned long address, int write)
{
return __follow_page(mm, address, 0, write, 1);
}
/*
* check_user_page_readable() can be called frm niterrupt context by oprofile,
* so we need to avoid taking any non-irq-safe locks
*/
int check_user_page_readable(struct mm_struct *mm, unsigned long address)
{
return __follow_page(mm, address, 1, 0, 0) != NULL;
}
EXPORT_SYMBOL(check_user_page_readable);
static inline int static inline int
untouched_anonymous_page(struct mm_struct* mm, struct vm_area_struct *vma, untouched_anonymous_page(struct mm_struct* mm, struct vm_area_struct *vma,
unsigned long address) unsigned long address)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment