linux-kernel - Re: [PATCH next] mm/thp: fix collapse_file()'s try_to

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: <Yh2enUcd2AMP4gq4@casper.infradead.org>
Date:   Tue, 1 Mar 2022 04:18:37 +0000
From:   Matthew Wilcox <willy@...radead.org>
To:     Hugh Dickins <hughd@...gle.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel@...r.kernel.org, linux-mm@...ck.org
Subject: Re: [PATCH next] mm/thp: fix collapse_file()'s try_to_unmap(folio,)

On Sat, Feb 26, 2022 at 06:22:47PM -0800, Hugh Dickins wrote:
> The foliation of THP collapse_file()'s call to try_to_unmap() is
> currently wrong, crashing on a test in rmap_walk() when xas_next()
> delivered a value (after which page has been loaded independently).

Argh.  I have a fear of this exact bug, and I must have missed checking
for it this time.  I hate trying to keep two variables in sync, so my
preferred fix for this is to remove it for this merge window:

+++ b/mm/khugepaged.c
@@ -1699,8 +1699,7 @@ static void collapse_file(struct mm_struct *mm,
 
        xas_set(&xas, start);
        for (index = start; index < end; index++) {
-               struct folio *folio = xas_next(&xas);
-               struct page *page = &folio->page;
+               struct page *page = xas_next(&xas);
 
                VM_BUG_ON(index != xas.xa_index);
                if (is_shmem) {
@@ -1835,7 +1834,8 @@ static void collapse_file(struct mm_struct *mm,
                }
 
                if (page_mapped(page))
-                       try_to_unmap(folio, TTU_IGNORE_MLOCK | TTU_BATCH_FLUSH);
+                       try_to_unmap(page_folio(page),
+                                       TTU_IGNORE_MLOCK | TTU_BATCH_FLUSH);
 
                xas_lock_irq(&xas);
                xas_set(&xas, index);

(ie revert the first hunk).  I'll come back to khugepaged in the next
merge window and convert this function properly.  It's going to take
some surgery to shmem in order to use folios there first ...