From: Will Deacon <will@kernel.org>
To: Nadav Amit <nadav.amit@gmail.com>
Cc: linux-mm@kvack.org, linux-kernel@vger.kernel.org,
Nadav Amit <namit@vmware.com>,
Andrea Arcangeli <aarcange@redhat.com>,
Yu Zhao <yuzhao@google.com>, Andy Lutomirski <luto@kernel.org>,
Peter Xu <peterx@redhat.com>, Pavel Emelyanov <xemul@openvz.org>,
Mike Kravetz <mike.kravetz@oracle.com>,
Mike Rapoport <rppt@linux.vnet.ibm.com>,
Minchan Kim <minchan@kernel.org>,
Peter Zijlstra <peterz@infradead.org>
Subject: Re: [RFC PATCH v2 2/2] fs/task_mmu: acquire mmap_lock for write on soft-dirty cleanup
Date: Tue, 5 Jan 2021 15:08:42 +0000 [thread overview]
Message-ID: <20210105150841.GA11745@willie-the-truck> (raw)
In-Reply-To: <20201225092529.3228466-3-namit@vmware.com>
On Fri, Dec 25, 2020 at 01:25:29AM -0800, Nadav Amit wrote:
> From: Nadav Amit <namit@vmware.com>
>
> Clearing soft-dirty through /proc/[pid]/clear_refs can cause memory
> corruption as it clears the dirty-bit without acquiring the mmap_lock
> for write and defers TLB flushes.
>
> As a result of this behavior, it is possible that one of the CPUs would
> have the stale PTE cached in its TLB and keep updating the page while
> another thread triggers a page-fault, and the page-fault handler would
> copy the old page into a new one.
[...]
> diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c
> index 217aa2705d5d..39b2bd27af79 100644
> --- a/fs/proc/task_mmu.c
> +++ b/fs/proc/task_mmu.c
> @@ -1189,6 +1189,7 @@ static ssize_t clear_refs_write(struct file *file, const char __user *buf,
> struct mm_struct *mm;
> struct vm_area_struct *vma;
> enum clear_refs_types type;
> + bool write_lock = false;
> struct mmu_gather tlb;
> int itype;
> int rv;
> @@ -1236,21 +1237,16 @@ static ssize_t clear_refs_write(struct file *file, const char __user *buf,
> }
> tlb_gather_mmu(&tlb, mm, 0, -1);
> if (type == CLEAR_REFS_SOFT_DIRTY) {
> + mmap_read_unlock(mm);
> + if (mmap_write_lock_killable(mm)) {
> + count = -EINTR;
> + goto out_mm;
> + }
> for (vma = mm->mmap; vma; vma = vma->vm_next) {
> - if (!(vma->vm_flags & VM_SOFTDIRTY))
> - continue;
> - mmap_read_unlock(mm);
> - if (mmap_write_lock_killable(mm)) {
> - count = -EINTR;
> - goto out_mm;
> - }
> - for (vma = mm->mmap; vma; vma = vma->vm_next) {
> - vma->vm_flags &= ~VM_SOFTDIRTY;
> - vma_set_page_prot(vma);
> - }
> - mmap_write_downgrade(mm);
> - break;
> + vma->vm_flags &= ~VM_SOFTDIRTY;
> + vma_set_page_prot(vma);
> }
> + write_lock = true;
>
> mmu_notifier_range_init(&range, MMU_NOTIFY_SOFT_DIRTY,
> 0, NULL, mm, 0, -1UL);
> @@ -1261,7 +1257,10 @@ static ssize_t clear_refs_write(struct file *file, const char __user *buf,
> if (type == CLEAR_REFS_SOFT_DIRTY)
> mmu_notifier_invalidate_range_end(&range);
> tlb_finish_mmu(&tlb, 0, -1);
> - mmap_read_unlock(mm);
> + if (write_lock)
> + mmap_write_unlock(mm);
> + else
> + mmap_read_unlock(mm);
> out_mm:
> mmput(mm);
I probably wouldn't bother with the 'write_lock' variable, and just check
'type == CLEAR_REFS_SOFT_DIRTY' instead.
But that's trivial and I don't have strong opinions, so:
Acked-by: Will Deacon <will@kernel.org>
Are you intending to land this for 5.11? If so, I can just rebase my other
series on top of this.
Will
next prev parent reply other threads:[~2021-01-05 15:09 UTC|newest]
Thread overview: 96+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-25 9:25 [RFC PATCH v2 0/2] mm: fix races due to deferred TLB flushes Nadav Amit
2020-12-25 9:25 ` [RFC PATCH v2 1/2] mm/userfaultfd: fix memory corruption due to writeprotect Nadav Amit
2021-01-04 12:22 ` Peter Zijlstra
2021-01-04 19:24 ` Andrea Arcangeli
2021-01-04 19:35 ` Nadav Amit
2021-01-04 20:19 ` Andrea Arcangeli
2021-01-04 20:39 ` Nadav Amit
2021-01-04 21:01 ` Andrea Arcangeli
2021-01-04 21:26 ` Nadav Amit
2021-01-05 18:45 ` Andrea Arcangeli
2021-01-05 19:05 ` Nadav Amit
2021-01-05 19:45 ` Andrea Arcangeli
2021-01-05 20:06 ` Nadav Amit
2021-01-05 21:06 ` Andrea Arcangeli
2021-01-05 21:43 ` Peter Xu
2021-01-05 8:13 ` Peter Zijlstra
2021-01-05 8:52 ` Nadav Amit
2021-01-05 14:26 ` Peter Zijlstra
2021-01-05 8:58 ` Peter Zijlstra
2021-01-05 9:22 ` Nadav Amit
2021-01-05 17:58 ` Andrea Arcangeli
2021-01-05 15:08 ` Peter Xu
2021-01-05 18:08 ` Andrea Arcangeli
2021-01-05 18:41 ` Peter Xu
2021-01-05 18:55 ` Andrea Arcangeli
2021-01-05 19:07 ` Nadav Amit
2021-01-05 19:43 ` Peter Xu
2020-12-25 9:25 ` [RFC PATCH v2 2/2] fs/task_mmu: acquire mmap_lock for write on soft-dirty cleanup Nadav Amit
2021-01-05 15:08 ` Will Deacon [this message]
2021-01-05 18:20 ` Andrea Arcangeli
2021-01-05 19:26 ` Nadav Amit
2021-01-05 20:39 ` Andrea Arcangeli
2021-01-05 21:20 ` Yu Zhao
2021-01-05 21:22 ` Nadav Amit
2021-01-05 22:16 ` Will Deacon
2021-01-06 0:29 ` Andrea Arcangeli
2021-01-06 0:02 ` Andrea Arcangeli
2021-01-07 20:04 ` [PATCH 0/2] page_count can't be used to decide when wp_page_copy Andrea Arcangeli
2021-01-07 20:04 ` [PATCH 1/2] mm: proc: Invalidate TLB after clearing soft-dirty page state Andrea Arcangeli
2021-01-07 20:04 ` [PATCH 2/2] mm: soft_dirty: userfaultfd: introduce wrprotect_tlb_flush_pending Andrea Arcangeli
2021-01-07 20:17 ` Linus Torvalds
2021-01-07 20:25 ` Linus Torvalds
2021-01-07 20:58 ` Andrea Arcangeli
2021-01-07 21:29 ` Linus Torvalds
2021-01-07 21:53 ` John Hubbard
2021-01-07 22:00 ` Linus Torvalds
2021-01-07 22:14 ` John Hubbard
2021-01-07 22:20 ` Linus Torvalds
2021-01-07 22:24 ` Linus Torvalds
2021-01-07 22:37 ` John Hubbard
2021-01-15 11:27 ` Jan Kara
2021-01-07 22:31 ` Andrea Arcangeli
2021-01-07 22:42 ` Linus Torvalds
2021-01-07 22:51 ` Linus Torvalds
2021-01-07 23:48 ` Andrea Arcangeli
2021-01-08 0:25 ` Linus Torvalds
2021-01-08 12:48 ` Will Deacon
2021-01-08 16:14 ` Andrea Arcangeli
2021-01-08 17:39 ` Linus Torvalds
2021-01-08 17:53 ` Andrea Arcangeli
2021-01-08 19:25 ` Linus Torvalds
2021-01-09 0:12 ` Andrea Arcangeli
2021-01-08 17:30 ` Linus Torvalds
2021-01-07 23:28 ` Andrea Arcangeli
2021-01-07 21:36 ` kernel test robot
2021-01-07 20:25 ` [PATCH 0/2] page_count can't be used to decide when wp_page_copy Jason Gunthorpe
2021-01-07 20:32 ` Linus Torvalds
2021-01-07 21:05 ` Linus Torvalds
2021-01-07 22:02 ` Andrea Arcangeli
2021-01-07 22:17 ` Linus Torvalds
2021-01-07 22:56 ` Andrea Arcangeli
2021-01-09 19:32 ` Matthew Wilcox
2021-01-09 19:46 ` Linus Torvalds
2021-01-15 14:30 ` Jan Kara
2021-01-07 21:54 ` Andrea Arcangeli
2021-01-07 21:45 ` Andrea Arcangeli
2021-01-08 13:36 ` Jason Gunthorpe
2021-01-08 17:00 ` Andrea Arcangeli
2021-01-08 18:19 ` Jason Gunthorpe
2021-01-08 18:31 ` Andy Lutomirski
2021-01-08 18:38 ` Linus Torvalds
2021-01-08 23:34 ` Andrea Arcangeli
2021-01-09 19:03 ` Andy Lutomirski
2021-01-09 19:15 ` Linus Torvalds
2021-01-08 18:59 ` Linus Torvalds
2021-01-08 22:43 ` Andrea Arcangeli
2021-01-09 0:42 ` Jason Gunthorpe
2021-01-09 2:50 ` Andrea Arcangeli
2021-01-11 14:30 ` Jason Gunthorpe
2021-01-13 21:56 ` Jerome Glisse
2021-01-13 23:39 ` Jason Gunthorpe
2021-01-14 2:35 ` Jerome Glisse
[not found] ` <20210109034958.6928-1-hdanton@sina.com>
2021-01-11 14:39 ` Jason Gunthorpe
2021-01-05 21:55 ` [RFC PATCH v2 2/2] fs/task_mmu: acquire mmap_lock for write on soft-dirty cleanup Peter Xu
2021-03-02 22:13 ` [RFC PATCH v2 0/2] mm: fix races due to deferred TLB flushes Peter Xu
2021-03-02 22:14 ` Nadav Amit
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=20210105150841.GA11745@willie-the-truck \
--to=will@kernel.org \
--cc=aarcange@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=luto@kernel.org \
--cc=mike.kravetz@oracle.com \
--cc=minchan@kernel.org \
--cc=nadav.amit@gmail.com \
--cc=namit@vmware.com \
--cc=peterx@redhat.com \
--cc=peterz@infradead.org \
--cc=rppt@linux.vnet.ibm.com \
--cc=xemul@openvz.org \
--cc=yuzhao@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 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).