Re: [patch 2/2] lockless get_user_pages

Nick Piggin
2008-05-26T01:40:52Z

On Sun, May 25, 2008 at 07:18:06PM +0200, Johannes Weiner wrote:
> Hi Nick,
>
> Nick Piggin <npiggin@suse.de> writes:
>
> > +static noinline int gup_pte_range(pmd_t pmd, unsigned long addr,
> > + unsigned long end, int write, struct page **pages, int *nr)
> > +{
> > + unsigned long mask;
> > + pte_t *ptep;
> > +
> > + mask = _PAGE_PRESENT|_PAGE_USER;
> > + if (write)
> > + mask |= _PAGE_RW;
> > +
> > + ptep = pte_offset_map(&pmd, addr);
> > + do {
> > + pte_t pte = gup_get_pte(ptep);
> > + struct page *page;
> > +
> > + if ((pte_val(pte) & (mask | _PAGE_SPECIAL)) != mask)
> > + return 0;
>
> Don't you leak the possbile high mapping here?

Hi Johannes,

Right you are. Good spotting.

--
Index: linux-2.6/arch/x86/mm/gup.c
===================================================================
--- linux-2.6.orig/arch/x86/mm/gup.c
+++ linux-2.6/arch/x86/mm/gup.c
@@ -80,8 +80,10 @@ static noinline int gup_pte_range(pmd_t
pte_t pte = gup_get_pte(ptep);
struct page *page;

- if ((pte_val(pte) & (mask | _PAGE_SPECIAL)) != mask)
+ if ((pte_val(pte) & (mask | _PAGE_SPECIAL)) != mask) {
+ pte_unmap(ptep);
return 0;
+ }
VM_BUG_ON(!pfn_valid(pte_pfn(pte)));
page = pte_page(pte);
get_page(page);