Commit be068f29 authored by Linus Torvalds's avatar Linus Torvalds

mm: fix misplaced unlock_page in do_wp_page()

Commit 09854ba9 ("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: 09854ba9 ("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: default avatarQian Cai <cai@redhat.com>
Reported-by: default avatarAlex Shi <alex.shi@linux.alibaba.com>
Bisected-and-analyzed-by: default avatarGerald Schaefer <gerald.schaefer@linux.ibm.com>
Tested-by: default avatarGerald Schaefer <gerald.schaefer@linux.ibm.com>
Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
parent c9c9e6a4
...@@ -2967,8 +2967,8 @@ static vm_fault_t do_wp_page(struct vm_fault *vmf) ...@@ -2967,8 +2967,8 @@ static vm_fault_t do_wp_page(struct vm_fault *vmf)
* page count reference, and the page is locked, * page count reference, and the page is locked,
* it's dark out, and we're wearing sunglasses. Hit it. * it's dark out, and we're wearing sunglasses. Hit it.
*/ */
wp_page_reuse(vmf);
unlock_page(page); unlock_page(page);
wp_page_reuse(vmf);
return VM_FAULT_WRITE; return VM_FAULT_WRITE;
} else if (unlikely((vma->vm_flags & (VM_WRITE|VM_SHARED)) == } else if (unlikely((vma->vm_flags & (VM_WRITE|VM_SHARED)) ==
(VM_WRITE|VM_SHARED))) { (VM_WRITE|VM_SHARED))) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment