All of lore.kernel.org
 help / color / mirror / Atom feed
From: Suren Baghdasaryan <surenb@google.com>
To: akpm@linux-foundation.org
Cc: willy@infradead.org, torvalds@linuxfoundation.org,
	 vegard.nossum@oracle.com, mpe@ellerman.id.au,
	Liam.Howlett@oracle.com,  lrh2000@pku.edu.cn, mgorman@suse.de,
	linux-mm@kvack.org,  linux-kernel@vger.kernel.org,
	kernel-team@android.com, surenb@google.com
Subject: [PATCH 1/3] mm: change vma_start_read to fail if VMA got detached from under it
Date: Tue, 20 Jun 2023 16:57:24 -0700	[thread overview]
Message-ID: <20230620235726.3873043-1-surenb@google.com> (raw)

Current implementation of vma_start_read() checks VMA for being locked
before taking vma->vm_lock and then checks that again. This mechanism
fails to detect a case when the VMA gets write-locked, modified and
unlocked after the first check but before the vma->vm_lock is obtained.
While this is not strictly a problem (vma_start_read would not produce
a false unlocked result), this allows it to successfully lock a VMA which
got detached from the VMA tree while vma_start_read was locking it.
New condition checks for any change in vma->vm_lock_seq after we obtain
vma->vm_lock and will cause vma_start_read() to fail if the above race
occurs.

Fixes: 5e31275cc997 ("mm: add per-VMA lock and helper functions to control it")
Suggested-by: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Suren Baghdasaryan <surenb@google.com>
---
 include/linux/mm.h | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 27ce77080c79..8410da79c570 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -639,23 +639,24 @@ static inline void vma_numab_state_free(struct vm_area_struct *vma) {}
  */
 static inline bool vma_start_read(struct vm_area_struct *vma)
 {
-	/* Check before locking. A race might cause false locked result. */
-	if (vma->vm_lock_seq == READ_ONCE(vma->vm_mm->mm_lock_seq))
+	int vm_lock_seq = READ_ONCE(vma->vm_lock_seq);
+
+	/*
+	 * Check if VMA is locked before taking vma->vm_lock. A race or
+	 * mm_lock_seq overflow might cause false locked result.
+	 */
+	if (vm_lock_seq == READ_ONCE(vma->vm_mm->mm_lock_seq))
 		return false;
 
 	if (unlikely(down_read_trylock(&vma->vm_lock->lock) == 0))
 		return false;
 
-	/*
-	 * Overflow might produce false locked result.
-	 * False unlocked result is impossible because we modify and check
-	 * vma->vm_lock_seq under vma->vm_lock protection and mm->mm_lock_seq
-	 * modification invalidates all existing locks.
-	 */
-	if (unlikely(vma->vm_lock_seq == READ_ONCE(vma->vm_mm->mm_lock_seq))) {
+	/* Fail if VMA was write-locked after we checked it earlier */
+	if (unlikely(vm_lock_seq != READ_ONCE(vma->vm_lock_seq))) {
 		up_read(&vma->vm_lock->lock);
 		return false;
 	}
+
 	return true;
 }
 
-- 
2.41.0.162.gfafddb0af9-goog



             reply	other threads:[~2023-06-20 23:57 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-20 23:57 Suren Baghdasaryan [this message]
2023-06-20 23:57 ` [PATCH 2/3] mm: change vma_start_read to fail to lock a detached VMA Suren Baghdasaryan
2023-06-20 23:57 ` [PATCH 3/3] mm: check for VMA being detached before destroying it Suren Baghdasaryan
2023-06-21  0:05   ` Suren Baghdasaryan
2023-06-21  2:15   ` kernel test robot
2023-06-21  7:01     ` Suren Baghdasaryan
2023-06-21  5:53   ` kernel test robot
2023-06-26 20:51 ` [PATCH 1/3] mm: change vma_start_read to fail if VMA got detached from under it 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=20230620235726.3873043-1-surenb@google.com \
    --to=surenb@google.com \
    --cc=Liam.Howlett@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lrh2000@pku.edu.cn \
    --cc=mgorman@suse.de \
    --cc=mpe@ellerman.id.au \
    --cc=torvalds@linuxfoundation.org \
    --cc=vegard.nossum@oracle.com \
    --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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.