[4/8] mm: fix use-after-free when anon vma name is used after vma is freed

Commit Message

Andrew Morton March 5, 2022, 4:28 a.m. UTC

From: Suren Baghdasaryan <[e-mail]>
Subject: mm: fix use-after-free when anon vma name is used after vma is freed

When adjacent vmas are being merged it can result in the vma that was
originally passed to madvise_update_vma being destroyed.  In the current
implementation, the name parameter passed to madvise_update_vma points
directly to vma->anon_name and it is used after the call to vma_merge.  In
the cases when vma_merge merges the original vma and destroys it, this
might result in UAF.  For that the original vma would have to hold the
anon_vma_name with the last reference.  The following vma would need to
contain a different anon_vma_name object with the same string.  Such
scenario is shown below:

madvise_vma_behavior(vma)
  madvise_update_vma(vma, ..., anon_name == vma->anon_name)
    vma_merge(vma)
      __vma_adjust(vma) <-- merges vma with adjacent one
        vm_area_free(vma) <-- frees the original vma
    replace_vma_anon_name(anon_name) <-- UAF of vma->anon_name

Fix this by raising the name refcount and stabilizing it.

Link: https://lkml.kernel.org/r/[e-mail]
Link: https://lkml.kernel.org/r/[e-mail]
Fixes: 9a10064f5625 ("mm: add a field to store names for private anonymous memory")
Signed-off-by: Suren Baghdasaryan <[e-mail]>
Reported-by: [e-mail]
Acked-by: Michal Hocko <[e-mail]>
Cc: Alexey Gladkov <[e-mail]>
Cc: Chris Hyser <[e-mail]>
Cc: Christian Brauner <[e-mail]>
Cc: Colin Cross <[e-mail]>
Cc: Cyrill Gorcunov <[e-mail]>
Cc: Dave Hansen <[e-mail]>
Cc: David Hildenbrand <[e-mail]>
Cc: Davidlohr Bueso <[e-mail]>
Cc: "Eric W. Biederman" <[e-mail]>
Cc: Johannes Weiner <[e-mail]>
Cc: Kees Cook <[e-mail]>
Cc: "Kirill A. Shutemov" <[e-mail]>
Cc: Matthew Wilcox <[e-mail]>
Cc: Michal Hocko <[e-mail]>
Cc: Peter Collingbourne <[e-mail]>
Cc: Sasha Levin <[e-mail]>
Cc: Sumit Semwal <[e-mail]>
Cc: Vlastimil Babka <[e-mail]>
Cc: Xiaofeng Cao <[e-mail]>
Signed-off-by: Andrew Morton <[e-mail]>
---

 mm/madvise.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

Patch

--- a/mm/madvise.c~mm-fix-use-after-free-when-anon-vma-name-is-used-after-vma-is-freed
+++ a/mm/madvise.c
@@ -131,6 +131,8 @@  static int replace_anon_vma_name(struct
 /*
  * Update the vm_flags on region of a vma, splitting it or merging it as
  * necessary.  Must be called with mmap_sem held for writing;
+ * Caller should ensure anon_name stability by raising its refcount even when
+ * anon_name belongs to a valid vma because this function might free that vma.
  */
 static int madvise_update_vma(struct vm_area_struct *vma,
 			      struct vm_area_struct **prev, unsigned long start,
@@ -945,6 +947,7 @@  static int madvise_vma_behavior(struct v
 				unsigned long behavior)
 {
 	int error;
+	struct anon_vma_name *anon_name;
 	unsigned long new_flags = vma->vm_flags;
 
 	switch (behavior) {
@@ -1010,8 +1013,11 @@  static int madvise_vma_behavior(struct v
 		break;
 	}
 
+	anon_name = anon_vma_name(vma);
+	anon_vma_name_get(anon_name);
 	error = madvise_update_vma(vma, prev, start, end, new_flags,
-				   anon_vma_name(vma));
+				   anon_name);
+	anon_vma_name_put(anon_name);
 
 out:
 	/*