linux-kernel - Re: [PATCH 1/2] mm/page_alloc: remove track of active PCP lists range in bulk free

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] [day] [month] [year] [list]

Message-ID: <CAF8kJuMvLG6NL_r0Lnf5Un5bskrG6P6i4OhOVKFuhBQ5f9D91g@mail.gmail.com>
Date:   Thu, 17 Aug 2023 00:51:23 -0700
From:   Chris Li <chrisl@...nel.org>
To:     Kemeng Shi <shikemeng@...weicloud.com>
Cc:     linux-mm@...ck.org, linux-kernel@...r.kernel.org,
        akpm@...ux-foundation.org, baolin.wang@...ux.alibaba.com,
        mgorman@...hsingularity.net, david@...hat.com, willy@...radead.org
Subject: Re: [PATCH 1/2] mm/page_alloc: remove track of active PCP lists range
 in bulk free

Hi Kemeng,

On Wed, Aug 16, 2023 at 7:22 PM Kemeng Shi <shikemeng@...weicloud.com> wrote:
> Hi Chris, there is no functional change intended in this patch. As
> I menthioned in changelog, there is no wrap for list iteration, so
> that the active PCP lists range will never be used.
> > I have a patch sitting in my tree for a while related to this
> > count vs pcp->count.  The BPF function hook can potentially change
> > pcp->count and make count out of sync with pcp->count which causes
> > a dead loop.

In this case the BPF allocates a page inside spin_lock. The "pcp->count" is
smaller than "count". The loop condition only checks "count > 0" but all
pcp->lists pages have been free. pcp->count is 0 but "count" is 1.
After a few times wrap around, the pindex_max is smaller than pindex_min,
then reach to -1 cause the invalid page fault.

> I guess pcp->count is set to bigger than it should be. In this case,
> we will keep trying get pages while all pages in pcp list were taken
> off already and dead lock will happen. In this case, dead looo will
> happen with or without this patch as the root cause is that we try
> to get pages more than pcp list owns.> Maybe I can send my out alone side with yours for discussion?

My patch is split into two parts, the first patch is a functional
change to allow
pcp->count drop below "count".
The other patch is just to clean up, and should have the same function.

Sure will send it out and CC you for discussion.

> > I don't mind my patch combined with yours.
> >
> Either way is acceptable to me, just feel free to choose one you like
> and I'd like to see if more we could do to this.

Thanks

Chris