linux-kernel - Re: [PATCH rfc v2 3/5] page_pool: add page recycling support based on elevated refcnt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKgT0Udnb_KgHyWiwxDF+r+DkytgZd4CQJz4QR85JpinhZAJzw@mail.gmail.com>
Date:   Sat, 10 Jul 2021 10:31:41 -0700
From:   Alexander Duyck <alexander.duyck@...il.com>
To:     Yunsheng Lin <linyunsheng@...wei.com>
Cc:     David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Russell King - ARM Linux <linux@...linux.org.uk>,
        Marcin Wojtas <mw@...ihalf.com>, linuxarm@...neuler.org,
        yisen.zhuang@...wei.com, Salil Mehta <salil.mehta@...wei.com>,
        thomas.petazzoni@...tlin.com, hawk@...nel.org,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        John Fastabend <john.fastabend@...il.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Will Deacon <will@...nel.org>,
        Matthew Wilcox <willy@...radead.org>,
        Vlastimil Babka <vbabka@...e.cz>, fenghua.yu@...el.com,
        guro@...com, Peter Xu <peterx@...hat.com>,
        Feng Tang <feng.tang@...el.com>,
        Jason Gunthorpe <jgg@...pe.ca>,
        Matteo Croce <mcroce@...rosoft.com>,
        Hugh Dickins <hughd@...gle.com>,
        Jonathan Lemon <jonathan.lemon@...il.com>,
        Alexander Lobakin <alobakin@...me>,
        Willem de Bruijn <willemb@...gle.com>, wenxu@...oud.cn,
        Cong Wang <cong.wang@...edance.com>,
        Kevin Hao <haokexin@...il.com>, nogikh@...gle.com,
        Marco Elver <elver@...gle.com>,
        Netdev <netdev@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>, bpf <bpf@...r.kernel.org>
Subject: Re: [PATCH rfc v2 3/5] page_pool: add page recycling support based on
 elevated refcnt

On Sat, Jul 10, 2021 at 12:44 AM Yunsheng Lin <linyunsheng@...wei.com> wrote:
<snip>
> @@ -419,6 +471,20 @@ static __always_inline struct page *
>  __page_pool_put_page(struct page_pool *pool, struct page *page,
>                      unsigned int dma_sync_size, bool allow_direct)
>  {
> +       int bias = page_pool_get_pagecnt_bias(page);
> +
> +       /* Handle the elevated refcnt case first */
> +       if (bias) {
> +               /* It is not the last user yet */
> +               if (!page_pool_bias_page_recyclable(page, bias))
> +                       return NULL;
> +
> +               if (likely(!page_is_pfmemalloc(page)))
> +                       goto recyclable;
> +               else
> +                       goto unrecyclable;
> +       }
> +

So this part is still broken. Anything that takes a reference to the
page and holds it while this is called will cause it to break. For
example with the recent fixes we put in place all it would take is a
skb_clone followed by pskb_expand_head and this starts leaking memory.

One of the key bits in order for pagecnt_bias to work is that you have
to deduct the bias once there are no more parties using it. Otherwise
you leave the reference count artificially inflated and the page will
never be freed. It works fine for the single producer single consumer
case but once you introduce multiple consumers this is going to fall
apart.