Messages in this thread Patch in this message | | | Date | Tue, 19 Jun 2012 21:47:12 +0200 | From | Oleg Nesterov <> | Subject | [PATCH 2/5] uprobes: __replace_page() should not use page_address_in_vma() |
| |
page_address_in_vma(old_page) in __replace_page() is ugly and wrong. The caller already knows the correct virtual address, this page was found by get_user_pages(vaddr).
However, page_address_in_vma() can actually fail if page->mapping was cleared by __delete_from_page_cache() after get_user_pages() returns. But this means the race with page reclaim, write_opcode() should not fail, it should retry and read this page again. Not sure this race is really possible though, page_freeze_refs() logic should prevent it.
We could change __replace_page() to return -EAGAIN in this case, but it would be better to simply use the caller's vaddr and rely on page_check_address().
Signed-off-by: Oleg Nesterov <oleg@redhat.com> --- kernel/events/uprobes.c | 10 +++------- 1 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c index a2b32a5..5b10705 100644 --- a/kernel/events/uprobes.c +++ b/kernel/events/uprobes.c @@ -132,17 +132,13 @@ static loff_t vma_address(struct vm_area_struct *vma, loff_t offset) * * Returns 0 on success, -EFAULT on failure. */ -static int __replace_page(struct vm_area_struct *vma, struct page *page, struct page *kpage) +static int __replace_page(struct vm_area_struct *vma, unsigned long addr, + struct page *page, struct page *kpage) { struct mm_struct *mm = vma->vm_mm; - unsigned long addr; spinlock_t *ptl; pte_t *ptep; - addr = page_address_in_vma(page, vma); - if (addr == -EFAULT) - return -EFAULT; - ptep = page_check_address(page, mm, addr, &ptl, 0); if (!ptep) return -EAGAIN; @@ -243,7 +239,7 @@ retry: goto unlock_out; lock_page(new_page); - ret = __replace_page(vma, old_page, new_page); + ret = __replace_page(vma, vaddr, old_page, new_page); unlock_page(new_page); unlock_out: -- 1.5.5.1
| |