The patch titled
Subject: mm: fix use-after-free when anon vma name is used after vma is freed
has been added to the -mm tree. Its filename is
mm-fix-use-after-free-when-anon-vma-name-is-used-after-vma-is-freed.patch
This patch should soon appear at
https://ozlabs.org/~akpm/mmots/broken-out/
and later at
https://ozlabs.org/~akpm/mmotm/broken-out/
Before you just go and hit "reply", please:
a) Consider who else should be cc'ed
b) Prefer to cc a suitable mailing list as well
c) Ideally: find the original patch on the mailing list and do a
reply-to-all to that, adding suitable additional cc's
*** Remember to use Documentation/process/submit-checklist.rst when testing your code ***
The -mm tree is included into linux-next and is updated
there every 3-4 working days
------------------------------------------------------
From: Suren Baghdasaryan <surenb@xxxxxxxxxx>
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->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 will result in use-after-free bug as shown below:
madvise_vma_behavior << passes vma->anon_name->name as name param
madvise_update_vma(name)
vma_merge
__vma_adjust
vm_area_free <-- frees the vma
replace_vma_anon_name(name) <-- UAF
Fix this by raising the name refcount and stabilizing it. Introduce
vma_anon_name_{get/put} API for this purpose.
Link: https://lkml.kernel.org/r/20220211013032.623763-1-surenb@xxxxxxxxxx
Fixes: 9a10064f5625 ("mm: add a field to store names for private anonymous memory")
Signed-off-by: Suren Baghdasaryan <surenb@xxxxxxxxxx>
Reported-by: syzbot+aa7b3d4b35f9dc46a366@xxxxxxxxxxxxxxxxxxxxxxxxx
Cc: Colin Cross <ccross@xxxxxxxxxx>
Cc: Sumit Semwal <sumit.semwal@xxxxxxxxxx>
Cc: Michal Hocko <mhocko@xxxxxxxx>
Cc: Dave Hansen <dave.hansen@xxxxxxxxx>
Cc: Kees Cook <keescook@xxxxxxxxxxxx>
Cc: Matthew Wilcox <willy@xxxxxxxxxxxxx>
Cc: Kirill A. Shutemov <kirill.shutemov@xxxxxxxxxxxxxxx>
Cc: Vlastimil Babka <vbabka@xxxxxxx>
Cc: Johannes Weiner <hannes@xxxxxxxxxxx>
Cc: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx>
Cc: Christian Brauner <brauner@xxxxxxxxxx>
Cc: Alexey Gladkov <legion@xxxxxxxxxx>
Cc: Sasha Levin <sashal@xxxxxxxxxx>
Cc: Chris Hyser <chris.hyser@xxxxxxxxxx>
Cc: Davidlohr Bueso <dave@xxxxxxxxxxxx>
Cc: Peter Collingbourne <pcc@xxxxxxxxxx>
Cc: Xiaofeng Cao <caoxiaofeng@xxxxxxxxxx>
Cc: David Hildenbrand <david@xxxxxxxxxx>
Cc: Cyrill Gorcunov <gorcunov@xxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
---
--- a/include/linux/mm_inline.h~mm-fix-use-after-free-when-anon-vma-name-is-used-after-vma-is-freed
+++ a/include/linux/mm_inline.h
@@ -145,6 +145,11 @@ static __always_inline void del_page_fro
*/
extern const char *vma_anon_name(struct vm_area_struct *vma);
+/* mmap_lock should be read-locked */
+extern struct anon_vma_name *vma_anon_name_get(struct vm_area_struct *vma);
+
+extern void vma_anon_name_put(struct anon_vma_name *anon_name);
+
/*
* mmap_lock should be read-locked for orig_vma->vm_mm.
* mmap_lock should be write-locked for new_vma->vm_mm or new_vma should be
@@ -176,6 +181,14 @@ static inline const char *vma_anon_name(
{
return NULL;
}
+
+static inline
+struct anon_vma_name *vma_anon_name_get(struct vm_area_struct *vma)
+{
+ return NULL;
+}
+
+static inline void vma_anon_name_put(struct anon_vma_name *anon_name) {}
static inline void dup_vma_anon_name(struct vm_area_struct *orig_vma,
struct vm_area_struct *new_vma) {}
static inline void free_vma_anon_name(struct vm_area_struct *vma) {}
--- a/mm/madvise.c~mm-fix-use-after-free-when-anon-vma-name-is-used-after-vma-is-freed
+++ a/mm/madvise.c
@@ -70,6 +70,9 @@ static struct anon_vma_name *anon_vma_na
struct anon_vma_name *anon_name;
size_t count;
+ if (!name)
+ return NULL;
+
/* Add 1 for NUL terminator at the end of the anon_name->name */
count = strlen(name) + 1;
anon_name = kmalloc(struct_size(anon_name, name, count), GFP_KERNEL);
@@ -103,6 +106,23 @@ const char *vma_anon_name(struct vm_area
return vma->anon_name->name;
}
+struct anon_vma_name *vma_anon_name_get(struct vm_area_struct *vma)
+{
+ if (!has_vma_anon_name(vma))
+ return NULL;
+
+ mmap_assert_locked(vma->vm_mm);
+
+ kref_get(&vma->anon_name->kref);
+ return vma->anon_name;
+}
+
+void vma_anon_name_put(struct anon_vma_name *anon_name)
+{
+ if (anon_name)
+ kref_put(&anon_name->kref, vma_anon_name_free);
+}
+
void dup_vma_anon_name(struct vm_area_struct *orig_vma,
struct vm_area_struct *new_vma)
{
@@ -126,33 +146,34 @@ void free_vma_anon_name(struct vm_area_s
}
/* mmap_lock should be write-locked */
-static int replace_vma_anon_name(struct vm_area_struct *vma, const char *name)
+static int replace_vma_anon_name(struct vm_area_struct *vma,
+ struct anon_vma_name *anon_name)
{
- const char *anon_name;
+ const char *orig_name;
- if (!name) {
+ if (!anon_name) {
free_vma_anon_name(vma);
return 0;
}
- anon_name = vma_anon_name(vma);
- if (anon_name) {
+ orig_name = vma_anon_name(vma);
+ if (orig_name) {
/* Same name, nothing to do here */
- if (!strcmp(name, anon_name))
+ if (!strcmp(anon_name->name, orig_name))
return 0;
free_vma_anon_name(vma);
}
- vma->anon_name = anon_vma_name_alloc(name);
- if (!vma->anon_name)
- return -ENOMEM;
+ kref_get(&anon_name->kref);
+ vma->anon_name = anon_name;
return 0;
}
#else /* CONFIG_ANON_VMA_NAME */
-static int replace_vma_anon_name(struct vm_area_struct *vma, const char *name)
+static int replace_vma_anon_name(struct vm_area_struct *vma,
+ struct anon_vma_name *anon_name)
{
- if (name)
+ if (anon_name)
return -EINVAL;
return 0;
@@ -161,12 +182,15 @@ static int replace_vma_anon_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,
unsigned long end, unsigned long new_flags,
- const char *name)
+ struct anon_vma_name *anon_name)
{
+ const char *name = anon_name ? anon_name->name : NULL;
struct mm_struct *mm = vma->vm_mm;
int error;
pgoff_t pgoff;
@@ -209,7 +233,7 @@ success:
*/
vma->vm_flags = new_flags;
if (!vma->vm_file) {
- error = replace_vma_anon_name(vma, name);
+ error = replace_vma_anon_name(vma, anon_name);
if (error)
return error;
}
@@ -976,6 +1000,7 @@ static int madvise_vma_behavior(struct v
{
int error;
unsigned long new_flags = vma->vm_flags;
+ struct anon_vma_name *anon_name;
switch (behavior) {
case MADV_REMOVE:
@@ -1040,8 +1065,10 @@ static int madvise_vma_behavior(struct v
break;
}
+ anon_name = vma_anon_name_get(vma);
error = madvise_update_vma(vma, prev, start, end, new_flags,
- vma_anon_name(vma));
+ anon_name);
+ vma_anon_name_put(anon_name);
out:
/*
@@ -1225,7 +1252,7 @@ int madvise_walk_vmas(struct mm_struct *
static int madvise_vma_anon_name(struct vm_area_struct *vma,
struct vm_area_struct **prev,
unsigned long start, unsigned long end,
- unsigned long name)
+ unsigned long anon_name)
{
int error;
@@ -1234,7 +1261,7 @@ static int madvise_vma_anon_name(struct
return -EBADF;
error = madvise_update_vma(vma, prev, start, end, vma->vm_flags,
- (const char *)name);
+ (struct anon_vma_name *)anon_name);
/*
* madvise() returns EAGAIN if kernel resources, such as
@@ -1248,8 +1275,10 @@ static int madvise_vma_anon_name(struct
int madvise_set_anon_name(struct mm_struct *mm, unsigned long start,
unsigned long len_in, const char *name)
{
+ struct anon_vma_name *anon_name;
unsigned long end;
unsigned long len;
+ int ret;
if (start & ~PAGE_MASK)
return -EINVAL;
@@ -1266,8 +1295,12 @@ int madvise_set_anon_name(struct mm_stru
if (end == start)
return 0;
- return madvise_walk_vmas(mm, start, end, (unsigned long)name,
+ anon_name = anon_vma_name_alloc(name);
+ ret = madvise_walk_vmas(mm, start, end, (unsigned long)anon_name,
madvise_vma_anon_name);
+ vma_anon_name_put(anon_name);
+
+ return ret;
}
#endif /* CONFIG_ANON_VMA_NAME */
/*
_
Patches currently in -mm which might be from surenb@xxxxxxxxxx are
mm-fix-use-after-free-when-anon-vma-name-is-used-after-vma-is-freed.patch
