linux-kernel - Re: [v3 PATCH 2/2] mm: thp: check page_mapped instead of page

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: <alpine.LSU.2.11.2105261523250.16407@eggly.anvils>
Date:   Wed, 26 May 2021 15:48:42 -0700 (PDT)
From:   Hugh Dickins <hughd@...gle.com>
To:     Yang Shi <shy828301@...il.com>
cc:     Hugh Dickins <hughd@...gle.com>, Zi Yan <ziy@...dia.com>,
        "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        HORIGUCHI NAOYA(堀口 直也) 
        <naoya.horiguchi@....com>, Wang Yugui <wangyugui@...-tech.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Linux MM <linux-mm@...ck.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [v3 PATCH 2/2] mm: thp: check page_mapped instead of page_mapcount
 for split

On Wed, 26 May 2021, Yang Shi wrote:
> On Tue, May 25, 2021 at 4:58 PM Hugh Dickins <hughd@...gle.com> wrote:
> > On Tue, 25 May 2021, Yang Shi wrote:
> > >
> > > We should be able to make dump_page() print total mapcount, right? The
> > > dump_page() should be just called in some error paths so taking some
> > > extra overhead to dump more information seems harmless, or am I
> > > missing something? Of course, this can be done in a separate patch.
> >
> > I didn't want to ask that of you, but yes, if you're willing to add
> > total_mapcount() into dump_page(), I think that would be ideal; and
> > could be helpful for other cases too.
> >
> > Looking through total_mapcount(), I think it's safe to call from
> > dump_page() - I always worry about extending crash info with
> > something that depends on a maybe-corrupted pointer which would
> > generate a further crash and either recurse or truncate the output -
> > but please check that carefully.
> 
> Yes, it is possible. If the THP is being split, some VM_BUG_* might be
> triggered if total_mapcount() is called. But it is still feasible to
> print total mapcount as long as we implement a more robust version for
> dump_page().

Oh dear. I think the very last thing the kernel needs is yet another
subtly different variant of *mapcount*().

Do you have a specific VM_BUG_* in mind there?  Of course there's
the VM_BUG_ON_PAGE(PageTail) at the start of it, and you'd want to
print total_mapcount(head) to avoid that one.

Looks like __dump_page() is already careful about "head", checking
whether "page" is within the expected bounds.  Of course, once we're
in serious VM_WARN territory, there might be races which could flip
fields midway: PageTail set by the time it reaches total_mapcount()?
Narrow the race (rather like it does with PageSlab) by testing
PageTail immediately before calling total_mapcount(head)?

Hugh