linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Suren Baghdasaryan <surenb@google.com>
To: stable@vger.kernel.org
Cc: gregkh@linuxfoundation.org, jannh@google.com,
	ktkhai@virtuozzo.com,  torvalds@linux-foundation.org,
	shli@fb.com, namit@vmware.com,  linux-mm@kvack.org,
	linux-kernel@vger.kernel.org, kernel-team@android.com,
	 Qian Cai <cai@redhat.com>, Alex Shi <alex.shi@linux.alibaba.com>,
	 Gerald Schaefer <gerald.schaefer@linux.ibm.com>
Subject: [PATCH 3/5] mm: fix misplaced unlock_page in do_wp_page()
Date: Thu,  1 Apr 2021 11:17:39 -0700	[thread overview]
Message-ID: <20210401181741.168763-4-surenb@google.com> (raw)
In-Reply-To: <20210401181741.168763-1-surenb@google.com>

From: Linus Torvalds <torvalds@linux-foundation.org>

Commit 09854ba94c6a ("mm: do_wp_page() simplification") reorganized all
the code around the page re-use vs copy, but in the process also moved
the final unlock_page() around to after the wp_page_reuse() call.

That normally doesn't matter - but it means that the unlock_page() is
now done after releasing the page table lock.  Again, not a big deal,
you'd think.

But it turns out that it's very wrong indeed, because once we've
released the page table lock, we've basically lost our only reference to
the page - the page tables - and it could now be free'd at any time.  We
do hold the mmap_sem, so no actual unmap() can happen, but madvise can
come in and a MADV_DONTNEED will zap the page range - and free the page.

So now the page may be free'd just as we're unlocking it, which in turn
will usually trigger a "Bad page state" error in the freeing path.  To
make matters more confusing, by the time the debug code prints out the
page state, the unlock has typically completed and everything looks fine
again.

This all doesn't happen in any normal situations, but it does trigger
with the dirtyc0w_child LTP test.  And it seems to trigger much more
easily (but not expclusively) on s390 than elsewhere, probably because
s390 doesn't do the "batch pages up for freeing after the TLB flush"
that gives the unlock_page() more time to complete and makes the race
harder to hit.

Fixes: 09854ba94c6a ("mm: do_wp_page() simplification")
Link: https://lore.kernel.org/lkml/a46e9bbef2ed4e17778f5615e818526ef848d791.camel@redhat.com/
Link: https://lore.kernel.org/linux-mm/c41149a8-211e-390b-af1d-d5eee690fecb@linux.alibaba.com/
Reported-by: Qian Cai <cai@redhat.com>
Reported-by: Alex Shi <alex.shi@linux.alibaba.com>
Bisected-and-analyzed-by: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
Tested-by: Gerald Schaefer <gerald.schaefer@linux.ibm.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
---
 mm/memory.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/memory.c b/mm/memory.c
index e84648d81d6d..14470ceaf3f2 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2848,8 +2848,8 @@ static int do_wp_page(struct vm_fault *vmf)
 		 * page count reference, and the page is locked,
 		 * it's dark out, and we're wearing sunglasses. Hit it.
 		 */
-		wp_page_reuse(vmf);
 		unlock_page(page);
+		wp_page_reuse(vmf);
 		return VM_FAULT_WRITE;
 	} else if (unlikely((vma->vm_flags & (VM_WRITE|VM_SHARED)) ==
 					(VM_WRITE|VM_SHARED))) {
-- 
2.31.0.291.g576ba9dcdaf-goog



  parent reply	other threads:[~2021-04-01 18:17 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-01 18:17 [PATCH 0/5] 4.14 backports of fixes for "CoW after fork() issue" Suren Baghdasaryan
2021-04-01 18:17 ` [PATCH 1/5] mm: reuse only-pte-mapped KSM page in do_wp_page() Suren Baghdasaryan
2021-04-01 19:38   ` Greg KH
2021-04-01 19:47     ` Suren Baghdasaryan
2021-04-01 18:17 ` [PATCH 2/5] mm: do_wp_page() simplification Suren Baghdasaryan
2021-04-01 18:17 ` Suren Baghdasaryan [this message]
2021-04-01 18:17 ` [PATCH 4/5] userfaultfd: wp: add helper for writeprotect check Suren Baghdasaryan
2021-04-01 18:17 ` [PATCH 5/5] mm/userfaultfd: fix memory corruption due to writeprotect Suren Baghdasaryan
2021-04-01 18:59 ` [PATCH 0/5] 4.14 backports of fixes for "CoW after fork() issue" Linus Torvalds
2021-04-01 19:43   ` Suren Baghdasaryan
2021-04-01 23:47     ` Peter Xu
2021-04-02  0:12       ` Suren Baghdasaryan
2021-04-07 13:21   ` Vlastimil Babka
2021-04-07 14:30     ` Peter Xu
2021-04-07 16:07     ` Linus Torvalds
2021-04-07 16:33       ` Suren Baghdasaryan
2021-04-07 17:04         ` Linus Torvalds
2021-04-07 18:47           ` Mikulas Patocka
2021-04-07 19:22             ` Linus Torvalds
2021-04-07 21:53               ` Suren Baghdasaryan
2021-04-21 20:01                 ` Suren Baghdasaryan
2021-04-21 21:05                   ` Peter Xu
2021-04-21 21:17                     ` Suren Baghdasaryan
2021-04-21 23:01                       ` Suren Baghdasaryan
2021-04-21 22:59                   ` Vlastimil Babka
2021-04-21 23:05                     ` Suren Baghdasaryan
2021-04-01 18:21 [PATCH 0/5] 4.19 " Suren Baghdasaryan
2021-04-01 18:21 ` [PATCH 3/5] mm: fix misplaced unlock_page in do_wp_page() Suren Baghdasaryan

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=20210401181741.168763-4-surenb@google.com \
    --to=surenb@google.com \
    --cc=alex.shi@linux.alibaba.com \
    --cc=cai@redhat.com \
    --cc=gerald.schaefer@linux.ibm.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jannh@google.com \
    --cc=kernel-team@android.com \
    --cc=ktkhai@virtuozzo.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=namit@vmware.com \
    --cc=shli@fb.com \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).