Re: Rare memory leakage

[Matthew Wilcox <willy@infradead.org>]

On Tue, Sep 22, 2020 at 12:44:51PM +0100, Matthew Wilcox wrote:
> I've just kicked off a test run using this:
> 
> +/*
> + * Have to be careful when freeing a non-compound allocation in case somebody
> + * else takes a temporary reference on the first page and then calls put_page()
> + */
>  void __free_pages(struct page *page, unsigned int order)
>  {
> -       if (put_page_testzero(page))
> -               free_the_page(page, order);
> +       if (likely(page_ref_freeze(page, 1)))
> +               goto free;
> +       if (likely(order == 0 || PageHead(page))) {
> +               if (put_page_testzero(page))
> +                       goto free;
> +               return;
> +       }
> +
> +       prep_compound_page(page, order);
> +       put_page(page);
> +       return;
> +free:
> +       free_the_page(page, order);
>  }
>  EXPORT_SYMBOL(__free_pages);
> 
> > > Better ideas?
> > 
> > IMHO, alloc_pages() with order > 0 and without __GFP_COMP is a weird beast. In
> > that case it would be probably best to refcount each base page separately. I
> > don't know how many assumptions this would break :/
> 
> You sound like a man who's never dealt with device drivers.  Multiorder,
> non-compound allocations are the norm in that world.

This seems easier to reason about:

+/*
+ * If we free a non-compound allocation, another thread may have a
+ * transient reference to the first page.  It has no way of knowing
+ * about the rest of the allocation, so we have to free all but the
+ * first page here.
+ */
 void __free_pages(struct page *page, unsigned int order)
 {
        if (put_page_testzero(page))
                free_the_page(page, order);
+       else
+               while (order-- > 0)
+                       free_the_page(page + 1 << order, order);
 }
 EXPORT_SYMBOL(__free_pages);