[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <4A780FE6.9050704@inria.fr>
Date: Tue, 04 Aug 2009 12:39:34 +0200
From: Brice Goglin <Brice.Goglin@...ia.fr>
To: Leon Woestenberg <leon.woestenberg@...il.com>
CC: Brice Goglin <Brice.Goglin@...ia.fr>,
Hugh Dickins <hugh.dickins@...cali.co.uk>,
linux-kernel@...r.kernel.org
Subject: [PATCH] mm: get_user_pages() stores ERR_PTR() in pages[i] on failure
>> I wonder if we should change get_user_pages to store ERR_PTR(ret)
>> in page[i] when it fails to get page #i.
>>
> Yes, I would see that as an improvement in finding out why rc <
> nr_pages, in case rc > 0.
>
> Also I think it does not break existing users.
>
Only compile-tested (and not in the nommu case).
When get_user_pages() fails to get a non-first page, it returns
the number of successfully gotten pages. The caller has to call
get_user_pages() again on the failed page to figure out the
error code.
Store the error code with ERR_PTR() in pages[i] when we fail
to get page #i.
The arch specific get_user_pages_fast() do not need to be changed
since they revert to the main get_user_pages() on failure.
Signed-off-by: Brice Goglin <Brice.Goglin@...ia.fr>
diff --git a/mm/memory.c b/mm/memory.c
index aede2ce..cd4efa3 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -1217,6 +1217,7 @@ int __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
int force = !!(flags & GUP_FLAGS_FORCE);
int ignore = !!(flags & GUP_FLAGS_IGNORE_VMA_PERMISSIONS);
int ignore_sigkill = !!(flags & GUP_FLAGS_IGNORE_SIGKILL);
+ int err = -EFAULT;
if (nr_pages <= 0)
return 0;
@@ -1243,7 +1244,7 @@ int __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
/* user gate pages are read-only */
if (!ignore && write)
- return i ? : -EFAULT;
+ goto abort;
if (pg > TASK_SIZE)
pgd = pgd_offset_k(pg);
else
@@ -1253,11 +1254,11 @@ int __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
BUG_ON(pud_none(*pud));
pmd = pmd_offset(pud, pg);
if (pmd_none(*pmd))
- return i ? : -EFAULT;
+ goto abort;
pte = pte_offset_map(pmd, pg);
if (pte_none(*pte)) {
pte_unmap(pte);
- return i ? : -EFAULT;
+ goto abort;
}
if (pages) {
struct page *page = vm_normal_page(gate_vma, start, *pte);
@@ -1277,7 +1278,7 @@ int __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
if (!vma ||
(vma->vm_flags & (VM_IO | VM_PFNMAP)) ||
(!ignore && !(vm_flags & vma->vm_flags)))
- return i ? : -EFAULT;
+ goto abort;
if (is_vm_hugetlb_page(vma)) {
i = follow_hugetlb_page(mm, vma, pages, vmas,
@@ -1302,8 +1303,10 @@ int __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
* we're only unlocking already resident/mapped pages.
*/
if (unlikely(!ignore_sigkill &&
- fatal_signal_pending(current)))
- return i ? i : -ERESTARTSYS;
+ fatal_signal_pending(current))) {
+ err = -ERESTARTSYS;
+ goto abort;
+ }
if (write)
foll_flags |= FOLL_WRITE;
@@ -1317,10 +1320,11 @@ int __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
FAULT_FLAG_WRITE : 0);
if (ret & VM_FAULT_ERROR) {
- if (ret & VM_FAULT_OOM)
- return i ? i : -ENOMEM;
- else if (ret & VM_FAULT_SIGBUS)
- return i ? i : -EFAULT;
+ if (ret & VM_FAULT_OOM) {
+ err = -ENOMEM;
+ goto abort;
+ } else if (ret & VM_FAULT_SIGBUS)
+ goto abort;
BUG();
}
if (ret & VM_FAULT_MAJOR)
@@ -1346,8 +1350,10 @@ int __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
cond_resched();
}
- if (IS_ERR(page))
- return i ? i : PTR_ERR(page);
+ if (IS_ERR(page)) {
+ err = PTR_ERR(page);
+ goto abort;
+ }
if (pages) {
pages[i] = page;
@@ -1362,6 +1368,11 @@ int __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
} while (nr_pages && start < vma->vm_end);
} while (nr_pages);
return i;
+
+ abort:
+ if (pages)
+ pages[i] = ERR_PTR(err);
+ return i ? : err;
}
/**
diff --git a/mm/nommu.c b/mm/nommu.c
index 53cab10..5fe8083 100644
--- a/mm/nommu.c
+++ b/mm/nommu.c
@@ -212,6 +212,8 @@ int __get_user_pages(struct task_struct *tsk, struct mm_struct *mm,
return i;
finish_or_fault:
+ if (pages)
+ pages[i] = ERR_PTR(-EFAULT);
return i ? : -EFAULT;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists