Re: [PATCH 2/2] hugepages: Fix use after free bug in "quota" handling

[David Gibson <dwg@au1.ibm.com>]

On Wed, Mar 07, 2012 at 08:28:39PM +0800, Hillf Danton wrote:
> On Wed, Mar 7, 2012 at 12:48 PM, David Gibson
> <david@gibson.dropbear.id.au> wrote:
[snip]
> >  /*
> >  * Region tracking -- allows tracking of reservations and instantiated pages
> >  *                    across the pages in a mapping.
> > @@ -533,9 +611,9 @@ static void free_huge_page(struct page *page)
> >         */
> >        struct hstate *h = page_hstate(page);
> >        int nid = page_to_nid(page);
> > -       struct address_space *mapping;
> > +       struct hugepage_subpool *spool =
> > +               (struct hugepage_subpool *)page_private(page);
> >
> > -       mapping = (struct address_space *) page_private(page);
> >        set_page_private(page, 0);
> >        page->mapping = NULL;
> >        BUG_ON(page_count(page));
> > @@ -551,8 +629,7 @@ static void free_huge_page(struct page *page)
> >                enqueue_huge_page(h, page);
> >        }
> >        spin_unlock(&hugetlb_lock);
> > -       if (mapping)
> > -               hugetlb_put_quota(mapping, 1);
> > +       hugepage_subpool_put_pages(spool, 1);
> 
> Like current code, quota is handed back *unconditionally*, but ...

[snip]
> >        /*
> > -        * Processes that did not create the mapping will have no reserves and
> > -        * will not have accounted against quota. Check that the quota can be
> > -        * made before satisfying the allocation
> > -        * MAP_NORESERVE mappings may also need pages and quota allocated
> > -        * if no reserve mapping overlaps.
> > +        * Processes that did not create the mapping will have no
> > +        * reserves and will not have accounted against subpool
> > +        * limit. Check that the subpool limit can be made before
> > +        * satisfying the allocation MAP_NORESERVE mappings may also
> > +        * need pages and subpool limit allocated allocated if no reserve
> > +        * mapping overlaps.
> >         */
> >        chg = vma_needs_reservation(h, vma, addr);
> >        if (chg < 0)
> >                return ERR_PTR(-VM_FAULT_OOM);
> >        if (chg)
> > -               if (hugetlb_get_quota(inode->i_mapping, chg))
> > +               if (hugepage_subpool_get_pages(spool, chg))
> >                        return ERR_PTR(-VM_FAULT_SIGBUS);
> 
> ... quota is allocated only if needed.
> 
> Given that mismatch, are you fixing the use-after-free bug, or
> quota maintenance, or both?

As you say, this is as before.  So I'm only fixing the use-after-free
bug.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson