linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
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, Suren Baghdasaryan <surenb@google.com>
Subject: [PATCH v2 5/6] mm: always lock new vma before inserting into vma tree
Date: Tue,  1 Aug 2023 15:07:31 -0700	[thread overview]
Message-ID: <20230801220733.1987762-6-surenb@google.com> (raw)
In-Reply-To: <20230801220733.1987762-1-surenb@google.com>

While it's not strictly necessary to lock a newly created vma before
adding it into the vma tree (as long as no further changes are performed
to it), it seems like a good policy to lock it and prevent accidental
changes after it becomes visible to the page faults. Lock the vma before
adding it into the vma tree.

Suggested-by: Jann Horn <jannh@google.com>
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
---
 mm/mmap.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 3937479d0e07..850a39dee075 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -412,6 +412,8 @@ static int vma_link(struct mm_struct *mm, struct vm_area_struct *vma)
 	if (vma_iter_prealloc(&vmi))
 		return -ENOMEM;
 
+	vma_start_write(vma);
+
 	if (vma->vm_file) {
 		mapping = vma->vm_file->f_mapping;
 		i_mmap_lock_write(mapping);
@@ -477,7 +479,8 @@ static inline void vma_prepare(struct vma_prepare *vp)
 	vma_start_write(vp->vma);
 	if (vp->adj_next)
 		vma_start_write(vp->adj_next);
-	/* vp->insert is always a newly created VMA, no need for locking */
+	if (vp->insert)
+		vma_start_write(vp->insert);
 	if (vp->remove)
 		vma_start_write(vp->remove);
 	if (vp->remove2)
@@ -3098,6 +3101,7 @@ static int do_brk_flags(struct vma_iterator *vmi, struct vm_area_struct *vma,
 	vma->vm_pgoff = addr >> PAGE_SHIFT;
 	vm_flags_init(vma, flags);
 	vma->vm_page_prot = vm_get_page_prot(flags);
+	vma_start_write(vma);
 	if (vma_iter_store_gfp(vmi, vma, GFP_KERNEL))
 		goto mas_store_fail;
 
@@ -3345,7 +3349,6 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap,
 			get_file(new_vma->vm_file);
 		if (new_vma->vm_ops && new_vma->vm_ops->open)
 			new_vma->vm_ops->open(new_vma);
-		vma_start_write(new_vma);
 		if (vma_link(mm, new_vma))
 			goto out_vma_link;
 		*need_rmap_locks = false;
-- 
2.41.0.585.gd2178a4bd4-goog



  parent reply	other threads:[~2023-08-01 22:07 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-08-01 22:07 [PATCH v2 0/6] make vma locking more obvious Suren Baghdasaryan
2023-08-01 22:07 ` [PATCH v2 1/6] mm: enable page walking API to lock vmas during the walk Suren Baghdasaryan
2023-08-01 22:07 ` [PATCH v2 2/6] mm: for !CONFIG_PER_VMA_LOCK equate write lock assertion for vma and mmap Suren Baghdasaryan
2023-08-02 17:07   ` Liam R. Howlett
2023-08-01 22:07 ` [PATCH v2 3/6] mm: replace mmap with vma write lock assertions when operating on a vma Suren Baghdasaryan
2023-08-02 17:13   ` Liam R. Howlett
2023-08-01 22:07 ` [PATCH v2 4/6] mm: lock vma explicitly before doing vm_flags_reset and vm_flags_reset_once Suren Baghdasaryan
2023-08-02 17:13   ` Liam R. Howlett
2023-08-02 17:49   ` Linus Torvalds
2023-08-02 18:09     ` Suren Baghdasaryan
2023-08-02 18:53       ` Linus Torvalds
2023-08-02 20:21         ` Suren Baghdasaryan
2023-08-01 22:07 ` Suren Baghdasaryan [this message]
2023-08-02 17:02   ` [PATCH v2 5/6] mm: always lock new vma before inserting into vma tree Liam R. Howlett
2023-08-01 22:07 ` [PATCH v2 6/6] mm: move vma locking out of vma_prepare Suren Baghdasaryan
2023-08-02 16:59   ` Liam R. Howlett
2023-08-02 17:24     ` Suren Baghdasaryan
2023-08-03 17:28 ` [PATCH v2 0/6] make vma locking more obvious 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=20230801220733.1987762-6-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=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