Re: [RFC PATCH] Add support for eXclusive Page Frame Ownership (XPFO)

[Laura Abbott]

On 03/21/2016 01:37 AM, Juerg Haefliger wrote:
...
> > > +void xpfo_free_page(struct page *page, int order)
> > > +{
> > > +    int i;
> > > +    unsigned long kaddr;
> > > +
> > > +    for (i = 0; i < (1 << order); i++) {
> > > +
> > > +        /* The page frame was previously allocated to user space */
> > > +        if (TEST_AND_CLEAR_XPFO_FLAG(user, page + i)) {
> > > +            kaddr = (unsigned long)page_address(page + i);
> > > +
> > > +            /* Clear the page and mark it accordingly */
> > > +            clear_page((void *)kaddr);
> >
> > Clearing the page isn't related to XPFO. There's other work ongoing to
> > do clearing of the page on free.
>
> It's not strictly related to XPFO but adds another layer of security. Do you
> happen to have a pointer to the ongoing work that you mentioned?

The work was merged for the 4.6 merge window
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=8823b1dbc05fab1a8bec275eeae4709257c2661d

This is a separate option to clear the page.

...

> > > @@ -2072,10 +2076,11 @@ void free_hot_cold_page(struct page *page, bool cold)
> > >        }
> > >
> > >        pcp = &this_cpu_ptr(zone->pageset)->pcp;
> > > -    if (!cold)
> > > +    if (!cold && !xpfo_test_kernel(page))
> > >            list_add(&page->lru, &pcp->lists[migratetype]);
> > >        else
> > >            list_add_tail(&page->lru, &pcp->lists[migratetype]);
> > > +
> >
> > What's the advantage of this?
>
> Allocating a page to userspace that was previously allocated to kernel space
> requires an expensive TLB shootdown. The above will put previously
> kernel-allocated pages in the cold page cache to postpone their allocation as
> long as possible to minimize TLB shootdowns.

That makes sense. You probably want to make this a separate commmit with
this explanation as the commit text.


> > >        pcp->count++;
> > >        if (pcp->count >= pcp->high) {
> > >            unsigned long batch = READ_ONCE(pcp->batch);
>
> Thanks for the review and comments! It's highly appreciated.
>
> ...Juerg
>
>
> > Thanks,
> > Laura