linux-kernel - Re: [PATCH RFC 0/2] mm/page_alloc: free_pcppages

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]

Date:   Tue, 22 Aug 2023 11:57:04 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Chris Li <chrisl@...nel.org>
Cc:     Mel Gorman <mgorman@...hsingularity.net>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Kemeng Shi <shikemeng@...weicloud.com>,
        baolin.wang@...ux.alibaba.com, Michal Hocko <mhocko@...e.com>,
        David Hildenbrand <david@...hat.com>,
        Matthew Wilcox <willy@...radead.org>,
        linux-mm <linux-mm@...ck.org>,
        Namhyung Kim <namhyung@...gle.com>,
        Greg Thelen <gthelen@...gle.com>,
        LKML <linux-kernel@...r.kernel.org>,
        John Sperbeck <jsperbeck@...gle.com>,
        Huang Ying <ying.huang@...el.com>,
        Alexei Starovoitov <ast@...nel.org>
Subject: Re: [PATCH RFC 0/2] mm/page_alloc: free_pcppages_bulk safeguard

On Tue, Aug 22, 2023 at 10:48 AM Chris Li <chrisl@...nel.org> wrote:
>
> Hi Mel,
>
> Adding Alexei to the discussion.
>
> On Mon, Aug 21, 2023 at 3:32 AM Mel Gorman <mgorman@...hsingularity.net> wrote:
> >
> > On Thu, Aug 17, 2023 at 11:05:22PM -0700, Chris Li wrote:
> > > In this patch series I want to safeguard
> > > the free_pcppage_bulk against change in the
> > > pcp->count outside of this function. e.g.
> > > by BPF program inject on the function tracepoint.
> > >
> > > I break up the patches into two seperate patches
> > > for the safeguard and clean up.
> > >
> > > Hopefully that is easier to review.
> > >
> > > Signed-off-by: Chris Li <chrisl@...nel.org>
> >
> > This sounds like a maintenance nightmare if internal state can be arbitrary
> > modified by a BPF program and still expected to work properly in all cases.
> > Every review would have to take into account "what if a BPF script modifies
> > state behind our back?"

Where did this concern come from?
Since when BPF can modify arbitrary state?

But I wasn't cc-ed on the original patch, so not sure what it attempts to do.
Maybe concern is valid.

> Thanks for the feedback.
>
> I agree that it is hard to support if we allow BPF to change any internal
> stage as a rule.  That is why it is a RFC. Would you consider it case
> by case basis?
> The kernel panic is bad, the first patch is actually very small. I can
> also change it
> to generate warnings if we detect the inconsistent state.

panic and warns because of bpf prog?!
bpf infra takes all the precaution to make sure that bpf progs
can never cause such damage.

>
> How about the second (clean up) patch or Keming's clean up version? I can modify
> it to take out the pcp->count if the verdict is just not supporting
> BPF changing internal
> state at all. I do wish to get rid of the pindex_min and pindex_max.
>
> Thanks
>
> Chris