Re: [fuse-devel] fuse: trying to steal weird page

[Matthew Wilcox <willy@xxxxxxxxxxxxx>]

On Mon, May 18, 2020 at 04:58:05PM +0200, Miklos Szeredi wrote:
> On Mon, May 18, 2020 at 4:48 PM Matthew Wilcox <willy@xxxxxxxxxxxxx> wrote:
> >
> > On Mon, May 18, 2020 at 02:45:02PM +0200, Miklos Szeredi wrote:
> 
> > > page_cache_pipe_buf_steal() calls remove_mapping() which calls
> > > page_ref_unfreeze(page, 1).  That sets the refcount to 1, right?
> > >
> > > What am I missing?
> >
> > find_get_entry() calling page_cache_get_speculative().
> >
> > In a previous allocation, this page belonged to the page cache.  Then it
> > was freed, but another thread is in the middle of a page cache lookup and
> > has already loaded the pointer.  It is now delayed by a few clock ticks.
> >
> > Now the page is allocated to FUSE, which calls page_ref_unfreeze().
> > And then the refcount gets bumped to 2 by page_cache_get_speculative().
> > find_get_entry() calls xas_reload() and discovers this page is no longer
> > at that index, so it calls put_page(), but in that narrow window, FUSE
> > checks the refcount and finds it's not 1.
> 
> What if that page_cache_get_speculative() happens just before
> page_ref_unfreeze()?  The speculative reference would be lost and on
> put_page() the page would be freed, even though fuse is still holding
> the pointer.

static inline void page_ref_unfreeze(struct page *page, int count)
{
        VM_BUG_ON_PAGE(page_count(page) != 0, page);
        VM_BUG_ON(count == 0);

        atomic_set_release(&page->_refcount, count);

static inline int __page_cache_add_speculative(struct page *page, int count)
{
        if (unlikely(!page_ref_add_unless(page, count, 0))) {
                return 0;