linux-kernel - Re: [PATCH v4 0/7] mm/mprotect: Fix dax puds

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: <ZrPoUgISLqlF-iEQ@x1n>
Date: Wed, 7 Aug 2024 17:34:10 -0400
From: Peter Xu <peterx@...hat.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	"Aneesh Kumar K . V" <aneesh.kumar@...ux.ibm.com>,
	Michael Ellerman <mpe@...erman.id.au>,
	Oscar Salvador <osalvador@...e.de>,
	Dan Williams <dan.j.williams@...el.com>,
	James Houghton <jthoughton@...gle.com>,
	Matthew Wilcox <willy@...radead.org>,
	Nicholas Piggin <npiggin@...il.com>,
	Rik van Riel <riel@...riel.com>, Dave Jiang <dave.jiang@...el.com>,
	x86@...nel.org, Ingo Molnar <mingo@...hat.com>,
	Rick P Edgecombe <rick.p.edgecombe@...el.com>,
	"Kirill A . Shutemov" <kirill@...temov.name>,
	linuxppc-dev@...ts.ozlabs.org,
	Mel Gorman <mgorman@...hsingularity.net>,
	Hugh Dickins <hughd@...gle.com>, Borislav Petkov <bp@...en8.de>,
	David Hildenbrand <david@...hat.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Vlastimil Babka <vbabka@...e.cz>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Christophe Leroy <christophe.leroy@...roup.eu>,
	Huang Ying <ying.huang@...el.com>
Subject: Re: [PATCH v4 0/7] mm/mprotect: Fix dax puds

On Wed, Aug 07, 2024 at 02:17:03PM -0700, Andrew Morton wrote:
> On Wed,  7 Aug 2024 15:48:04 -0400 Peter Xu <peterx@...hat.com> wrote:
> 
> > 
> > Dax supports pud pages for a while, but mprotect on puds was missing since
> > the start.  This series tries to fix that by providing pud handling in
> > mprotect().  The goal is to add more types of pud mappings like hugetlb or
> > pfnmaps.  This series paves way for it by fixing known pud entries.
> > 
> > Considering nobody reported this until when I looked at those other types
> > of pud mappings, I am thinking maybe it doesn't need to be a fix for stable
> > and this may not need to be backported.  I would guess whoever cares about
> > mprotect() won't care 1G dax puds yet, vice versa.  I hope fixing that in
> > new kernels would be fine, but I'm open to suggestions.
> 
> Yes, I'm not sure this is a "fix" at all.  We're implementing something
> which previously wasn't there.  Perhaps the entire series should be
> called "mm: implement mprotect() for DAX PUDs"?

The problem is mprotect() will skip the dax 1G PUD while it shouldn't;
meanwhile it'll dump some bad PUD in dmesg.  Both of them look like (corner
case) bugs to me.. where:

  - skipping the 1G pud means mprotect() will succeed even if the pud won't
    be updated with the correct permission specified. Logically that can
    cause e.g. in mprotect(RO) then write the page can cause data corrupt,
    as the pud page will still be writable.

  - the bad pud will generate a pr_err() into dmesg, with no limit so far I
    can see.  So I think it means an userspace can DoS the kernel log if it
    wants.. simply by creating the PUD and keep mprotect-ing it

But yeah this series fixes this "bug" by implementing that part..

Thanks,

-- 
Peter Xu