All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kiryl Shutsemau <kas@kernel.org>
To: "David Hildenbrand (Arm)" <david@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	Zi Yan <ziy@nvidia.com>,
	 Baolin Wang <baolin.wang@linux.alibaba.com>,
	Hugh Dickins <hughd@google.com>,
	 "Liam R. Howlett" <liam@infradead.org>,
	Nico Pache <nico.pache@linux.dev>,
	 Ryan Roberts <ryan.roberts@arm.com>, Dev Jain <dev.jain@arm.com>,
	Barry Song <baohua@kernel.org>,
	 Lance Yang <lance.yang@linux.dev>,
	Usama Arif <usama.arif@linux.dev>,
	 Zach O'Keefe <zokeefe@google.com>,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	 Chris Mason <mason@kernel.org>,
	stable@vger.kernel.org
Subject: Re: [PATCH] mm/khugepaged: don't map folios that are not uptodate in MADV_COLLAPSE
Date: Fri, 6 Nov 2026 13:41:09 +0000	[thread overview]
Message-ID: <au3YD92l6BwCQIPJ@thinkstation> (raw)
In-Reply-To: <254c6f0a-9353-4806-a5ab-a071cd59c64f@kernel.org>

On Fri, Nov 06, 2026 at 01:29:31PM +0100, David Hildenbrand (Arm) wrote:
> > Separately from this fix: nothing on the mapping side catches this.
> > Every caller checks uptodate on its own (next_uptodate_folio(),
> > filemap_fault(), shmem_get_folio()), and khugepaged was the one that
> > went straight from filemap_lock_folio() to do_set_pmd().
> > 
> > Should do_set_pmd() and set_pte_range() assert it? Something like
> > 
> >         VM_WARN_ON_ONCE_FOLIO(folio->mapping && !folio_test_anon(folio) &&
> >                               !folio_test_uptodate(folio), folio);
> 
> I think so. But looking at these hard-to-trigger but in the end very nasty
> problems, should we actually reject the operation and do a pr_warn_once() or
> something?

Yes, rejecting sounds better.

Not in set_pte_range() as I suggested, though. Fault-around only gets
there with folios that next_uptodate_folio() checked under the folio
lock, and for CoW it maps the new anon folio after do_cow_fault() has
already copied from the page cache one.

Checking the folio ->fault() returned in finish_fault() covers PTE, PMD
and CoW; do_set_pmd() covers khugepaged. Both return VM_FAULT_SIGBUS.

Something like this, only compile-tested:

diff --git a/mm/memory.c b/mm/memory.c
index 6fe0b7dab927..ff2f2deff761 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5674,6 +5674,22 @@ static vm_fault_t __do_fault(struct vm_fault *vmf)
        return ret;
 }

+/*
+ * A page cache folio that is not uptodate holds data that was never read in
+ * or cleared, and mapping it leaks stale memory to userspace. The callers are
+ * expected to check, so treat a hit as a kernel bug and refuse the mapping.
+ */
+static bool file_folio_not_uptodate(struct folio *folio)
+{
+       if (likely(folio_test_uptodate(folio)))
+               return false;
+       /* Driver pages from ->fault() never get PG_uptodate */
+       if (folio_test_anon(folio) || !folio->mapping)
+               return false;
+       WARN_ONCE(1, "refusing to map a page cache folio that is not uptodate\n");
+       return true;
+}
+
 #ifdef CONFIG_TRANSPARENT_HUGEPAGE
 static void deposit_prealloc_pte(struct vm_fault *vmf)
 {
@@ -5714,6 +5730,9 @@ vm_fault_t do_set_pmd(struct vm_fault *vmf, struct folio *folio, struct page *pa
                return ret;
        page = &folio->page;

+       if (file_folio_not_uptodate(folio))
+               return VM_FAULT_SIGBUS;
+
        /*
         * Just backoff if any subpage of a THP is corrupted otherwise
         * the corrupted page may mapped by PMD silently to escape the
@@ -5849,6 +5868,16 @@ vm_fault_t finish_fault(struct vm_fault *vmf)
        unsigned long addr;
        bool needs_fallback = false;

+       /*
+        * Check the folio ->fault() returned. For CoW that is the source of the
+        * copy; DAX CoW has none.
+        */
+       if (vmf->page) {
+               folio = page_folio(vmf->page);
+               if (file_folio_not_uptodate(folio))
+                       return VM_FAULT_SIGBUS;
+       }
+
 fallback:
        addr = vmf->address;
-- 
  Kiryl Shutsemau / Kirill A. Shutemov


  reply	other threads:[~2026-11-06 13:41 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-05 18:16 [PATCH] mm/khugepaged: don't map folios that are not uptodate in MADV_COLLAPSE David Hildenbrand (Arm)
2026-11-05 19:34 ` Lorenzo Stoakes (ARM)
2026-11-05 19:58 ` Hugh Dickins
2026-11-05 20:09 ` Zi Yan
2026-11-05 21:24   ` Zach O'Keefe
2026-11-06  1:08 ` SJ Park
2026-11-06  2:49 ` Baolin Wang
2026-11-06  2:53 ` Lance Yang
2026-11-06 12:09 ` Kiryl Shutsemau
2026-11-06 12:29   ` David Hildenbrand (Arm)
2026-11-06 13:41     ` Kiryl Shutsemau [this message]
2026-11-06 13:54       ` David Hildenbrand (Arm)
2026-11-06 14:06     ` Pedro Falcato
2026-11-06 14:19       ` David Hildenbrand (Arm)
2026-11-06 12:49 ` David Hildenbrand
2026-11-06 13:50 ` Pedro Falcato
2026-11-07 19:33 ` David Hildenbrand

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=au3YD92l6BwCQIPJ@thinkstation \
    --to=kas@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=baohua@kernel.org \
    --cc=baolin.wang@linux.alibaba.com \
    --cc=david@kernel.org \
    --cc=dev.jain@arm.com \
    --cc=hughd@google.com \
    --cc=lance.yang@linux.dev \
    --cc=liam@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mason@kernel.org \
    --cc=nico.pache@linux.dev \
    --cc=ryan.roberts@arm.com \
    --cc=stable@vger.kernel.org \
    --cc=usama.arif@linux.dev \
    --cc=ziy@nvidia.com \
    --cc=zokeefe@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.