From: Suren Baghdasaryan <surenb@google.com>
To: akpm@linux-foundation.org
Cc: torvalds@linux-foundation.org, jannh@google.com,
willy@infradead.org, liam.howlett@oracle.com, david@redhat.com,
peterx@redhat.com, ldufour@linux.ibm.com, vbabka@suse.cz,
michel@lespinasse.org, jglisse@google.com, mhocko@suse.com,
hannes@cmpxchg.org, dave@stgolabs.net, hughd@google.com,
linux-kernel@vger.kernel.org, linux-mm@kvack.org,
stable@vger.kernel.org, kernel-team@android.com,
Suren Baghdasaryan <surenb@google.com>
Subject: [PATCH v4 0/6] make vma locking more obvious
Date: Fri, 4 Aug 2023 08:27:18 -0700 [thread overview]
Message-ID: <20230804152724.3090321-1-surenb@google.com> (raw)
During recent vma locking patch reviews Linus and Jann Horn noted a number
of issues with vma locking and suggested improvements:
1. walk_page_range() does not have ability to write-lock a vma during the
walk when it's done under mmap_write_lock. For example s390_reset_cmma().
2. Vma locking is hidden inside vm_flags modifiers and is hard to follow.
Suggestion is to change vm_flags_reset{_once} to assert that vma is
write-locked and require an explicit locking.
3. Same issue with vma_prepare() hiding vma locking.
4. In userfaultfd vm_flags are modified after vma->vm_userfaultfd_ctx and
page faults can operate on a context while it's changed.
5. do_brk_flags() and __install_special_mapping() not locking a newly
created vma before adding it into the mm. While not strictly a problem,
this is fragile if vma is modified after insertion, as in the
mmap_region() case which was recently fixed. Suggestion is to always lock
a new vma before inserting it and making it visible to page faults.
6. vma_assert_write_locked() for CONFIG_PER_VMA_LOCK=n would benefit from
being mmap_assert_write_locked() instead of no-op and then any place which
operates on a vma and calls mmap_assert_write_locked() can be converted
into vma_assert_write_locked().
I CC'ed stable only on the first patch because others are cleanups and the
bug in userfaultfd does not affect stable (lock_vma_under_rcu prevents
uffds from being handled under vma lock protection). However I would be
happy if the whole series is merged into stable 6.4 since it makes vma
locking more maintainable.
The patches apply cleanly over Linus' ToT and will conflict when applied
over mm-unstable due to missing [1]. The conflict can be easily resolved
by ignoring conflicting deletions but probably simpler to take [1] into
mm-unstable and avoid later conflict.
[1] commit 6c21e066f925 ("mm/mempolicy: Take VMA lock before replacing policy")
Changes since v3:
- changed vma locking in vma_merge to avoid locking prev when not
necessary, per Liam
Suren Baghdasaryan (6):
mm: enable page walking API to lock vmas during the walk
mm: for !CONFIG_PER_VMA_LOCK equate write lock assertion for vma and
mmap
mm: replace mmap with vma write lock assertions when operating on a
vma
mm: lock vma explicitly before doing vm_flags_reset and
vm_flags_reset_once
mm: always lock new vma before inserting into vma tree
mm: move vma locking out of vma_prepare and dup_anon_vma
arch/powerpc/kvm/book3s_hv_uvmem.c | 1 +
arch/powerpc/mm/book3s64/subpage_prot.c | 1 +
arch/riscv/mm/pageattr.c | 1 +
arch/s390/mm/gmap.c | 5 ++++
fs/proc/task_mmu.c | 5 ++++
fs/userfaultfd.c | 6 +++++
include/linux/mm.h | 13 ++++++---
include/linux/pagewalk.h | 11 ++++++++
mm/damon/vaddr.c | 2 ++
mm/hmm.c | 1 +
mm/hugetlb.c | 2 +-
mm/khugepaged.c | 5 ++--
mm/ksm.c | 25 ++++++++++-------
mm/madvise.c | 8 +++---
mm/memcontrol.c | 2 ++
mm/memory-failure.c | 1 +
mm/memory.c | 2 +-
mm/mempolicy.c | 22 +++++++++------
mm/migrate_device.c | 1 +
mm/mincore.c | 1 +
mm/mlock.c | 4 ++-
mm/mmap.c | 33 +++++++++++++++--------
mm/mprotect.c | 2 ++
mm/pagewalk.c | 36 ++++++++++++++++++++++---
mm/vmscan.c | 1 +
25 files changed, 148 insertions(+), 43 deletions(-)
--
2.41.0.585.gd2178a4bd4-goog
next reply other threads:[~2023-08-04 15:27 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-04 15:27 Suren Baghdasaryan [this message]
2023-08-04 15:27 ` [PATCH v4 1/6] mm: enable page walking API to lock vmas during the walk Suren Baghdasaryan
2023-08-04 19:14 ` Andrew Morton
2023-08-04 19:41 ` Suren Baghdasaryan
2023-08-04 15:27 ` [PATCH v4 2/6] mm: for !CONFIG_PER_VMA_LOCK equate write lock assertion for vma and mmap Suren Baghdasaryan
2023-08-04 15:27 ` [PATCH v4 3/6] mm: replace mmap with vma write lock assertions when operating on a vma Suren Baghdasaryan
2023-08-04 15:27 ` [PATCH v4 4/6] mm: lock vma explicitly before doing vm_flags_reset and vm_flags_reset_once Suren Baghdasaryan
2023-08-04 15:27 ` [PATCH v4 5/6] mm: always lock new vma before inserting into vma tree Suren Baghdasaryan
2023-08-14 14:54 ` Jann Horn
2023-08-14 19:15 ` Andrew Morton
2023-08-14 19:19 ` Liam R. Howlett
2023-08-14 20:02 ` Jann Horn
2023-08-14 20:06 ` Suren Baghdasaryan
2023-08-04 15:27 ` [PATCH v4 6/6] mm: move vma locking out of vma_prepare and dup_anon_vma 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=20230804152724.3090321-1-surenb@google.com \
--to=surenb@google.com \
--cc=akpm@linux-foundation.org \
--cc=dave@stgolabs.net \
--cc=david@redhat.com \
--cc=hannes@cmpxchg.org \
--cc=hughd@google.com \
--cc=jannh@google.com \
--cc=jglisse@google.com \
--cc=kernel-team@android.com \
--cc=ldufour@linux.ibm.com \
--cc=liam.howlett@oracle.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=michel@lespinasse.org \
--cc=peterx@redhat.com \
--cc=stable@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
--cc=vbabka@suse.cz \
--cc=willy@infradead.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