On Thu, Sep 02, 2021 at 04:18:12PM -0700, Suren Baghdasaryan wrote: > From: Colin Cross <ccross@xxxxxxxxxx> > > In many userspace applications, and especially in VM based applications > like Android uses heavily, there are multiple different allocators in use. > At a minimum there is libc malloc and the stack, and in many cases there > are libc malloc, the stack, direct syscalls to mmap anonymous memory, and > multiple VM heaps (one for small objects, one for big objects, etc.). > Each of these layers usually has its own tools to inspect its usage; > malloc by compiling a debug version, the VM through heap inspection tools, > and for direct syscalls there is usually no way to track them. > > On Android we heavily use a set of tools that use an extended version of > the logic covered in Documentation/vm/pagemap.txt to walk all pages mapped > in userspace and slice their usage by process, shared (COW) vs. unique > mappings, backing, etc. This can account for real physical memory usage > even in cases like fork without exec (which Android uses heavily to share > as many private COW pages as possible between processes), Kernel SamePage > Merging, and clean zero pages. It produces a measurement of the pages > that only exist in that process (USS, for unique), and a measurement of > the physical memory usage of that process with the cost of shared pages > being evenly split between processes that share them (PSS). > > If all anonymous memory is indistinguishable then figuring out the real > physical memory usage (PSS) of each heap requires either a pagemap walking > tool that can understand the heap debugging of every layer, or for every > layer's heap debugging tools to implement the pagemap walking logic, in > which case it is hard to get a consistent view of memory across the whole > system. > > Tracking the information in userspace leads to all sorts of problems. > It either needs to be stored inside the process, which means every > process has to have an API to export its current heap information upon > request, or it has to be stored externally in a filesystem that > somebody needs to clean up on crashes. It needs to be readable while > the process is still running, so it has to have some sort of > synchronization with every layer of userspace. Efficiently tracking > the ranges requires reimplementing something like the kernel vma > trees, and linking to it from every layer of userspace. It requires > more memory, more syscalls, more runtime cost, and more complexity to > separately track regions that the kernel is already tracking. > > This patch adds a field to /proc/pid/maps and /proc/pid/smaps to show a > userspace-provided name for anonymous vmas. The names of named anonymous > vmas are shown in /proc/pid/maps and /proc/pid/smaps as [anon:<name>]. > > Userspace can set the name for a region of memory by calling > prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, start, len, (unsigned long)name); > Setting the name to NULL clears it. The name length limit is 256 bytes > including NUL-terminator and is checked to contain only printable ascii > characters (including space), except '[','\' and ']'. Is the reason this isn't done via madvise() because we're forced into an "int" argument there? (Otherwise we could pass a pointer there.) int madvise(void *addr, size_t length, int advice); > > The name is stored in a pointer in the shared union in vm_area_struct > that points to a null terminated string. Anonymous vmas with the same > name (equivalent strings) and are otherwise mergeable will be merged. > The name pointers are not shared between vmas even if they contain the > same name. The name pointer is stored in a union with fields that are > only used on file-backed mappings, so it does not increase memory usage. > > The patch is based on the original patch developed by Colin Cross, more > specifically on its latest version [1] posted upstream by Sumit Semwal. > It used a userspace pointer to store vma names. In that design, name > pointers could be shared between vmas. However during the last upstreaming > attempt, Kees Cook raised concerns [2] about this approach and suggested > to copy the name into kernel memory space, perform validity checks [3] > and store as a string referenced from vm_area_struct. > One big concern is about fork() performance which would need to strdup > anonymous vma names. Dave Hansen suggested experimenting with worst-case > scenario of forking a process with 64k vmas having longest possible names > [4]. I ran this experiment on an ARM64 Android device and recorded a > worst-case regression of almost 40% when forking such a process. This > regression is addressed in the followup patch which replaces the pointer > to a name with a refcounted structure that allows sharing the name pointer > between vmas of the same name. Instead of duplicating the string during > fork() or when splitting a vma it increments the refcount. > > [1] https://lore.kernel.org/linux-mm/20200901161459.11772-4-sumit.semwal@xxxxxxxxxx/ > [2] https://lore.kernel.org/linux-mm/202009031031.D32EF57ED@keescook/ > [3] https://lore.kernel.org/linux-mm/202009031022.3834F692@keescook/ > [4] https://lore.kernel.org/linux-mm/5d0358ab-8c47-2f5f-8e43-23b89d6a8e95@xxxxxxxxx/ > > Changes for prctl(2) manual page (in the options section): > > PR_SET_VMA > Sets an attribute specified in arg2 for virtual memory areas > starting from the address specified in arg3 and spanning the > size specified in arg4. arg5 specifies the value of the attribute > to be set. Note that assigning an attribute to a virtual memory > area might prevent it from being merged with adjacent virtual > memory areas due to the difference in that attribute's value. > > Currently, arg2 must be one of: > > PR_SET_VMA_ANON_NAME > Set a name for anonymous virtual memory areas. arg5 should > be a pointer to a null-terminated string containing the > name. The name length including null byte cannot exceed > 256 bytes. If arg5 is NULL, the name of the appropriate > anonymous virtual memory areas will be reset. > > Signed-off-by: Colin Cross <ccross@xxxxxxxxxx> > [surenb: rebased over v5.14-rc7, replaced userpointer with a kernel copy > and added input sanitization. The bulk of the work here was done by Colin > Cross, therefore, with his permission, keeping him as the author] > Signed-off-by: Suren Baghdasaryan <surenb@xxxxxxxxxx> > --- > previous version including cover letter with test results is at: > https://lore.kernel.org/linux-mm/20210827191858.2037087-1-surenb@xxxxxxxxxx/ > > changes in v9 > - Added documentation for prctl(2) manual page describing newly introduced > options, per Pavel Machek > - Documented the downside of naming an anonymous vma which might prevent > it from being merged with adjacent vmas, per Cyrill Gorcunov > - Replaced seq_puts+seq_write with seq_printf, per Kees Cook > - Changed name validation to allow only printable ascii characters, except for > '[', '\' and ']', per Rasmus Villemoes > - Added madvise_set_anon_name definition dependency on CONFIG_PROC_FS, > per Michal Hocko > - Added NULL check for the name input in prctl_set_vma to correctly handle this > case, per Michal Hocko > - Handle the possibility of kstrdup returning NULL, per Rolf Eike Beer > - Changed max anon vma name length from 64 to 256 (as in the original patch) > because I found one case of the name length being 139 bytes. If anyone is > curious, here it is: > dalvik-/data/dalvik-cache/arm64/apex@com.android.permission@priv-app@GooglePermissionController@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx@classes.art > > Documentation/filesystems/proc.rst | 2 + > fs/proc/task_mmu.c | 12 ++- > fs/userfaultfd.c | 7 +- > include/linux/mm.h | 13 ++- > include/linux/mm_types.h | 48 ++++++++++- > include/uapi/linux/prctl.h | 3 + > kernel/fork.c | 2 + > kernel/sys.c | 61 ++++++++++++++ > mm/madvise.c | 131 ++++++++++++++++++++++++++--- > mm/mempolicy.c | 3 +- > mm/mlock.c | 2 +- > mm/mmap.c | 38 +++++---- > mm/mprotect.c | 2 +- > 13 files changed, 283 insertions(+), 41 deletions(-) > > diff --git a/Documentation/filesystems/proc.rst b/Documentation/filesystems/proc.rst > index 042c418f4090..a067eec54ef1 100644 > --- a/Documentation/filesystems/proc.rst > +++ b/Documentation/filesystems/proc.rst > @@ -431,6 +431,8 @@ is not associated with a file: > [stack] the stack of the main process > [vdso] the "virtual dynamic shared object", > the kernel system call handler > +[anon:<name>] an anonymous mapping that has been > + named by userspace > ======= ==================================== > > or if empty, the mapping is anonymous. > diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c > index eb97468dfe4c..d41edb4b4540 100644 > --- a/fs/proc/task_mmu.c > +++ b/fs/proc/task_mmu.c > @@ -308,6 +308,8 @@ show_map_vma(struct seq_file *m, struct vm_area_struct *vma) > > name = arch_vma_name(vma); > if (!name) { > + const char *anon_name; > + > if (!mm) { > name = "[vdso]"; > goto done; > @@ -319,8 +321,16 @@ show_map_vma(struct seq_file *m, struct vm_area_struct *vma) > goto done; > } > > - if (is_stack(vma)) > + if (is_stack(vma)) { > name = "[stack]"; > + goto done; > + } > + > + anon_name = vma_anon_name(vma); > + if (anon_name) { > + seq_pad(m, ' '); > + seq_printf(m, "[anon:%s]", anon_name); > + } > } > > done: > diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c > index 5c2d806e6ae5..5057843fb71a 100644 > --- a/fs/userfaultfd.c > +++ b/fs/userfaultfd.c > @@ -876,7 +876,7 @@ static int userfaultfd_release(struct inode *inode, struct file *file) > new_flags, vma->anon_vma, > vma->vm_file, vma->vm_pgoff, > vma_policy(vma), > - NULL_VM_UFFD_CTX); > + NULL_VM_UFFD_CTX, vma_anon_name(vma)); > if (prev) > vma = prev; > else > @@ -1440,7 +1440,8 @@ static int userfaultfd_register(struct userfaultfd_ctx *ctx, > prev = vma_merge(mm, prev, start, vma_end, new_flags, > vma->anon_vma, vma->vm_file, vma->vm_pgoff, > vma_policy(vma), > - ((struct vm_userfaultfd_ctx){ ctx })); > + ((struct vm_userfaultfd_ctx){ ctx }), > + vma_anon_name(vma)); > if (prev) { > vma = prev; > goto next; > @@ -1617,7 +1618,7 @@ static int userfaultfd_unregister(struct userfaultfd_ctx *ctx, > prev = vma_merge(mm, prev, start, vma_end, new_flags, > vma->anon_vma, vma->vm_file, vma->vm_pgoff, > vma_policy(vma), > - NULL_VM_UFFD_CTX); > + NULL_VM_UFFD_CTX, vma_anon_name(vma)); > if (prev) { > vma = prev; > goto next; > diff --git a/include/linux/mm.h b/include/linux/mm.h > index e59646a5d44d..c72226215f33 100644 > --- a/include/linux/mm.h > +++ b/include/linux/mm.h > @@ -2550,7 +2550,7 @@ static inline int vma_adjust(struct vm_area_struct *vma, unsigned long start, > extern struct vm_area_struct *vma_merge(struct mm_struct *, > struct vm_area_struct *prev, unsigned long addr, unsigned long end, > unsigned long vm_flags, struct anon_vma *, struct file *, pgoff_t, > - struct mempolicy *, struct vm_userfaultfd_ctx); > + struct mempolicy *, struct vm_userfaultfd_ctx, const char *); > extern struct anon_vma *find_mergeable_anon_vma(struct vm_area_struct *); > extern int __split_vma(struct mm_struct *, struct vm_area_struct *, > unsigned long addr, int new_below); > @@ -3285,5 +3285,16 @@ static inline int seal_check_future_write(int seals, struct vm_area_struct *vma) > return 0; > } > > +#if defined(CONFIG_ADVISE_SYSCALLS) && defined(CONFIG_PROC_FS) > +int madvise_set_anon_name(struct mm_struct *mm, unsigned long start, > + unsigned long len_in, const char *name); > +#else > +static inline int > +madvise_set_anon_name(struct mm_struct *mm, unsigned long start, > + unsigned long len_in, const char *name) { > + return 0; > +} > +#endif > + > #endif /* __KERNEL__ */ > #endif /* _LINUX_MM_H */ > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h > index 7f8ee09c711f..968a1d0463d8 100644 > --- a/include/linux/mm_types.h > +++ b/include/linux/mm_types.h > @@ -350,11 +350,19 @@ struct vm_area_struct { > /* > * For areas with an address space and backing store, > * linkage into the address_space->i_mmap interval tree. > + * > + * For private anonymous mappings, a pointer to a null terminated string > + * containing the name given to the vma, or NULL if unnamed. > */ > - struct { > - struct rb_node rb; > - unsigned long rb_subtree_last; > - } shared; > + > + union { > + struct { > + struct rb_node rb; > + unsigned long rb_subtree_last; > + } shared; > + /* Serialized by mmap_sem. */ > + char *anon_name; > + }; > > /* > * A file's MAP_PRIVATE vma can be in both i_mmap tree and anon_vma > @@ -809,4 +817,36 @@ typedef struct { > unsigned long val; > } swp_entry_t; > > +/* > + * mmap_lock should be read-locked when calling vma_anon_name() and while using > + * the returned pointer. > + */ > +extern const char *vma_anon_name(struct vm_area_struct *vma); > + > +/* > + * 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 > + * isolated. > + */ > +extern void dup_vma_anon_name(struct vm_area_struct *orig_vma, > + struct vm_area_struct *new_vma); > + > +/* > + * mmap_lock should be write-locked or vma should have been isolated under > + * write-locked mmap_lock protection. > + */ > +extern void free_vma_anon_name(struct vm_area_struct *vma); > + > +/* mmap_lock should be read-locked */ > +static inline bool is_same_vma_anon_name(struct vm_area_struct *vma, > + const char *name) > +{ > + const char *vma_name = vma_anon_name(vma); > + > + if (likely(!vma_name)) > + return name == NULL; > + > + return name && !strcmp(name, vma_name); > +} > + > #endif /* _LINUX_MM_TYPES_H */ > diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h > index 43bd7f713c39..4c8cbf510b2d 100644 > --- a/include/uapi/linux/prctl.h > +++ b/include/uapi/linux/prctl.h > @@ -269,4 +269,7 @@ struct prctl_mm_map { > # define PR_SCHED_CORE_SHARE_FROM 3 /* pull core_sched cookie to pid */ > # define PR_SCHED_CORE_MAX 4 > > +#define PR_SET_VMA 0x53564d41 > +# define PR_SET_VMA_ANON_NAME 0 > + > #endif /* _LINUX_PRCTL_H */ > diff --git a/kernel/fork.c b/kernel/fork.c > index 695d1343a254..cfb8c47564d8 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -366,12 +366,14 @@ struct vm_area_struct *vm_area_dup(struct vm_area_struct *orig) > *new = data_race(*orig); > INIT_LIST_HEAD(&new->anon_vma_chain); > new->vm_next = new->vm_prev = NULL; > + dup_vma_anon_name(orig, new); > } > return new; > } > > void vm_area_free(struct vm_area_struct *vma) > { > + free_vma_anon_name(vma); > kmem_cache_free(vm_area_cachep, vma); > } > > diff --git a/kernel/sys.c b/kernel/sys.c > index 72c7639e3c98..25118902a376 100644 > --- a/kernel/sys.c > +++ b/kernel/sys.c > @@ -2299,6 +2299,64 @@ int __weak arch_prctl_spec_ctrl_set(struct task_struct *t, unsigned long which, > > #define PR_IO_FLUSHER (PF_MEMALLOC_NOIO | PF_LOCAL_THROTTLE) > > +#ifdef CONFIG_MMU > + > +#define ANON_VMA_NAME_MAX_LEN 256 > + > +static inline bool is_valid_name_char(char ch) > +{ > + /* printable ascii characters, except [ \ ] */ > + return (ch > 0x1f && ch < 0x5b) || (ch > 0x5d && ch < 0x7f); > +} > + > +static int prctl_set_vma(unsigned long opt, unsigned long addr, > + unsigned long size, unsigned long arg) > +{ > + struct mm_struct *mm = current->mm; > + const char __user *uname; > + char *name, *pch; > + int error; > + > + switch (opt) { > + case PR_SET_VMA_ANON_NAME: > + uname = (const char __user *)arg; > + if (!uname) { > + /* Reset the name */ > + name = NULL; > + goto set_name; > + } > + > + name = strndup_user(uname, ANON_VMA_NAME_MAX_LEN); > + > + if (IS_ERR(name)) > + return PTR_ERR(name); > + > + for (pch = name; *pch != '\0'; pch++) { > + if (!is_valid_name_char(*pch)) { > + kfree(name); > + return -EINVAL; > + } > + } > +set_name: > + mmap_write_lock(mm); > + error = madvise_set_anon_name(mm, addr, size, name); > + mmap_write_unlock(mm); > + kfree(name); > + break; > + default: > + error = -EINVAL; > + } > + > + return error; > +} > +#else /* CONFIG_MMU */ > +static int prctl_set_vma(unsigned long opt, unsigned long start, > + unsigned long size, unsigned long arg) > +{ > + return -EINVAL; > +} > +#endif > + > SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3, > unsigned long, arg4, unsigned long, arg5) > { > @@ -2568,6 +2626,9 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3, > error = sched_core_share_pid(arg2, arg3, arg4, arg5); > break; > #endif > + case PR_SET_VMA: > + error = prctl_set_vma(arg2, arg3, arg4, arg5); > + break; > default: > error = -EINVAL; > break; > diff --git a/mm/madvise.c b/mm/madvise.c > index 54bf9f73f95d..0c6d0f64d432 100644 > --- a/mm/madvise.c > +++ b/mm/madvise.c > @@ -18,6 +18,7 @@ > #include <linux/fadvise.h> > #include <linux/sched.h> > #include <linux/sched/mm.h> > +#include <linux/string.h> > #include <linux/uio.h> > #include <linux/ksm.h> > #include <linux/fs.h> > @@ -62,19 +63,78 @@ static int madvise_need_mmap_write(int behavior) > } > } > > +static inline bool has_vma_anon_name(struct vm_area_struct *vma) > +{ > + return !vma->vm_file && vma->anon_name; > +} > + > +const char *vma_anon_name(struct vm_area_struct *vma) > +{ > + if (!has_vma_anon_name(vma)) > + return NULL; > + > + mmap_assert_locked(vma->vm_mm); > + > + return vma->anon_name; > +} > + > +void dup_vma_anon_name(struct vm_area_struct *orig_vma, > + struct vm_area_struct *new_vma) > +{ > + if (!has_vma_anon_name(orig_vma)) > + return; > + > + new_vma->anon_name = kstrdup(orig_vma->anon_name, GFP_KERNEL); > +} > + > +void free_vma_anon_name(struct vm_area_struct *vma) > +{ > + if (!has_vma_anon_name(vma)) > + return; > + > + kfree(vma->anon_name); > + vma->anon_name = NULL; > +} > + > +/* mmap_lock should be write-locked */ > +static int replace_vma_anon_name(struct vm_area_struct *vma, const char *name) > +{ > + if (!name) { > + free_vma_anon_name(vma); > + return 0; > + } > + > + if (vma->anon_name) { > + /* Should never happen, to dup use dup_vma_anon_name() */ > + WARN_ON(vma->anon_name == name); > + > + /* Same name, nothing to do here */ > + if (!strcmp(name, vma->anon_name)) > + return 0; > + > + free_vma_anon_name(vma); > + } > + vma->anon_name = kstrdup(name, GFP_KERNEL); > + if (!vma->anon_name) > + return -ENOMEM; > + > + return 0; > +} > + > /* > - * Update the vm_flags on regiion of a vma, splitting it or merging it as > + * 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; > */ > 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) > + unsigned long end, unsigned long new_flags, > + const char *name) > { > struct mm_struct *mm = vma->vm_mm; > int error; > pgoff_t pgoff; > > - if (new_flags == vma->vm_flags) { > + if (new_flags == vma->vm_flags && is_same_vma_anon_name(vma, name)) { > *prev = vma; > return 0; > } > @@ -82,7 +142,7 @@ static int madvise_update_vma(struct vm_area_struct *vma, > pgoff = vma->vm_pgoff + ((start - vma->vm_start) >> PAGE_SHIFT); > *prev = vma_merge(mm, *prev, start, end, new_flags, vma->anon_vma, > vma->vm_file, pgoff, vma_policy(vma), > - vma->vm_userfaultfd_ctx); > + vma->vm_userfaultfd_ctx, name); > if (*prev) { > vma = *prev; > goto success; > @@ -91,20 +151,16 @@ static int madvise_update_vma(struct vm_area_struct *vma, > *prev = vma; > > if (start != vma->vm_start) { > - if (unlikely(mm->map_count >= sysctl_max_map_count)) { > - error = -ENOMEM; > - return error; > - } > + if (unlikely(mm->map_count >= sysctl_max_map_count)) > + return -ENOMEM; > error = __split_vma(mm, vma, start, 1); > if (error) > return error; > } > > if (end != vma->vm_end) { > - if (unlikely(mm->map_count >= sysctl_max_map_count)) { > - error = -ENOMEM; > - return error; > - } > + if (unlikely(mm->map_count >= sysctl_max_map_count)) > + return -ENOMEM; > error = __split_vma(mm, vma, end, 0); > if (error) > return error; > @@ -115,10 +171,33 @@ static int madvise_update_vma(struct vm_area_struct *vma, > * vm_flags is protected by the mmap_lock held in write mode. > */ > vma->vm_flags = new_flags; > + if (!vma->vm_file) { > + error = replace_vma_anon_name(vma, name); > + if (error) > + return error; > + } > > return 0; > } > > +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) > +{ > + int error; > + > + /* Only anonymous mappings can be named */ > + if (vma->vm_file) > + return -EINVAL; To distinguish from the other EINVALs, should this maybe be EBADF? (As in "no, you can't do that, there is already an fd associated".) > + > + error = madvise_update_vma(vma, prev, start, end, vma->vm_flags, > + (const char *)name); > + if (error == -ENOMEM) > + error = -EAGAIN; I think a comment would be useful here too. AIUI, this is done to match the error behavior seen under madvise_vma_behavior(). > + return error; > +} > + > #ifdef CONFIG_SWAP > static int swapin_walk_pmd_entry(pmd_t *pmd, unsigned long start, > unsigned long end, struct mm_walk *walk) > @@ -942,7 +1021,8 @@ static int madvise_vma_behavior(struct vm_area_struct *vma, > break; > } > > - error = madvise_update_vma(vma, prev, start, end, new_flags); > + error = madvise_update_vma(vma, prev, start, end, new_flags, > + vma_anon_name(vma)); > > out: > /* > @@ -1121,6 +1201,31 @@ int madvise_walk_vmas(struct mm_struct *mm, unsigned long start, > return unmapped_error; > } > > +int madvise_set_anon_name(struct mm_struct *mm, unsigned long start, > + unsigned long len_in, const char *name) > +{ > + unsigned long end; > + unsigned long len; > + > + if (start & ~PAGE_MASK) > + return -EINVAL; > + len = (len_in + ~PAGE_MASK) & PAGE_MASK; > + > + /* Check to see whether len was rounded up from small -ve to zero */ > + if (len_in && !len) > + return -EINVAL; > + > + end = start + len; > + if (end < start) > + return -EINVAL; > + > + if (end == start) > + return 0; > + > + return madvise_walk_vmas(mm, start, end, (unsigned long)name, > + madvise_vma_anon_name); > +} > + > /* > * The madvise(2) system call. > * > diff --git a/mm/mempolicy.c b/mm/mempolicy.c > index e32360e90274..cc21ca7e9d40 100644 > --- a/mm/mempolicy.c > +++ b/mm/mempolicy.c > @@ -811,7 +811,8 @@ static int mbind_range(struct mm_struct *mm, unsigned long start, > ((vmstart - vma->vm_start) >> PAGE_SHIFT); > prev = vma_merge(mm, prev, vmstart, vmend, vma->vm_flags, > vma->anon_vma, vma->vm_file, pgoff, > - new_pol, vma->vm_userfaultfd_ctx); > + new_pol, vma->vm_userfaultfd_ctx, > + vma_anon_name(vma)); > if (prev) { > vma = prev; > next = vma->vm_next; > diff --git a/mm/mlock.c b/mm/mlock.c > index 16d2ee160d43..c878515680af 100644 > --- a/mm/mlock.c > +++ b/mm/mlock.c > @@ -511,7 +511,7 @@ static int mlock_fixup(struct vm_area_struct *vma, struct vm_area_struct **prev, > pgoff = vma->vm_pgoff + ((start - vma->vm_start) >> PAGE_SHIFT); > *prev = vma_merge(mm, *prev, start, end, newflags, vma->anon_vma, > vma->vm_file, pgoff, vma_policy(vma), > - vma->vm_userfaultfd_ctx); > + vma->vm_userfaultfd_ctx, vma_anon_name(vma)); > if (*prev) { > vma = *prev; > goto success; > diff --git a/mm/mmap.c b/mm/mmap.c > index 181a113b545d..c13934d41f65 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -1032,7 +1032,8 @@ int __vma_adjust(struct vm_area_struct *vma, unsigned long start, > */ > static inline int is_mergeable_vma(struct vm_area_struct *vma, > struct file *file, unsigned long vm_flags, > - struct vm_userfaultfd_ctx vm_userfaultfd_ctx) > + struct vm_userfaultfd_ctx vm_userfaultfd_ctx, > + const char *anon_name) > { > /* > * VM_SOFTDIRTY should not prevent from VMA merging, if we > @@ -1050,6 +1051,8 @@ static inline int is_mergeable_vma(struct vm_area_struct *vma, > return 0; > if (!is_mergeable_vm_userfaultfd_ctx(vma, vm_userfaultfd_ctx)) > return 0; > + if (!is_same_vma_anon_name(vma, anon_name)) > + return 0; > return 1; > } > > @@ -1082,9 +1085,10 @@ static int > can_vma_merge_before(struct vm_area_struct *vma, unsigned long vm_flags, > struct anon_vma *anon_vma, struct file *file, > pgoff_t vm_pgoff, > - struct vm_userfaultfd_ctx vm_userfaultfd_ctx) > + struct vm_userfaultfd_ctx vm_userfaultfd_ctx, > + const char *anon_name) > { > - if (is_mergeable_vma(vma, file, vm_flags, vm_userfaultfd_ctx) && > + if (is_mergeable_vma(vma, file, vm_flags, vm_userfaultfd_ctx, anon_name) && > is_mergeable_anon_vma(anon_vma, vma->anon_vma, vma)) { > if (vma->vm_pgoff == vm_pgoff) > return 1; > @@ -1103,9 +1107,10 @@ static int > can_vma_merge_after(struct vm_area_struct *vma, unsigned long vm_flags, > struct anon_vma *anon_vma, struct file *file, > pgoff_t vm_pgoff, > - struct vm_userfaultfd_ctx vm_userfaultfd_ctx) > + struct vm_userfaultfd_ctx vm_userfaultfd_ctx, > + const char *anon_name) Nit: one too many spaces before "const". > { > - if (is_mergeable_vma(vma, file, vm_flags, vm_userfaultfd_ctx) && > + if (is_mergeable_vma(vma, file, vm_flags, vm_userfaultfd_ctx, anon_name) && > is_mergeable_anon_vma(anon_vma, vma->anon_vma, vma)) { > pgoff_t vm_pglen; > vm_pglen = vma_pages(vma); > @@ -1116,9 +1121,9 @@ can_vma_merge_after(struct vm_area_struct *vma, unsigned long vm_flags, > } > > /* > - * Given a mapping request (addr,end,vm_flags,file,pgoff), figure out > - * whether that can be merged with its predecessor or its successor. > - * Or both (it neatly fills a hole). > + * Given a mapping request (addr,end,vm_flags,file,pgoff,anon_name), > + * figure out whether that can be merged with its predecessor or its > + * successor. Or both (it neatly fills a hole). > * > * In most cases - when called for mmap, brk or mremap - [addr,end) is > * certain not to be mapped by the time vma_merge is called; but when > @@ -1163,7 +1168,8 @@ struct vm_area_struct *vma_merge(struct mm_struct *mm, > unsigned long end, unsigned long vm_flags, > struct anon_vma *anon_vma, struct file *file, > pgoff_t pgoff, struct mempolicy *policy, > - struct vm_userfaultfd_ctx vm_userfaultfd_ctx) > + struct vm_userfaultfd_ctx vm_userfaultfd_ctx, > + const char *anon_name) > { > pgoff_t pglen = (end - addr) >> PAGE_SHIFT; > struct vm_area_struct *area, *next; > @@ -1193,7 +1199,7 @@ struct vm_area_struct *vma_merge(struct mm_struct *mm, > mpol_equal(vma_policy(prev), policy) && > can_vma_merge_after(prev, vm_flags, > anon_vma, file, pgoff, > - vm_userfaultfd_ctx)) { > + vm_userfaultfd_ctx, anon_name)) { > /* > * OK, it can. Can we now merge in the successor as well? > */ > @@ -1202,7 +1208,7 @@ struct vm_area_struct *vma_merge(struct mm_struct *mm, > can_vma_merge_before(next, vm_flags, > anon_vma, file, > pgoff+pglen, > - vm_userfaultfd_ctx) && > + vm_userfaultfd_ctx, anon_name) && > is_mergeable_anon_vma(prev->anon_vma, > next->anon_vma, NULL)) { > /* cases 1, 6 */ > @@ -1225,7 +1231,7 @@ struct vm_area_struct *vma_merge(struct mm_struct *mm, > mpol_equal(policy, vma_policy(next)) && > can_vma_merge_before(next, vm_flags, > anon_vma, file, pgoff+pglen, > - vm_userfaultfd_ctx)) { > + vm_userfaultfd_ctx, anon_name)) { > if (prev && addr < prev->vm_end) /* case 4 */ > err = __vma_adjust(prev, prev->vm_start, > addr, prev->vm_pgoff, NULL, next); > @@ -1760,7 +1766,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr, > * Can we just expand an old mapping? > */ > vma = vma_merge(mm, prev, addr, addr + len, vm_flags, > - NULL, file, pgoff, NULL, NULL_VM_UFFD_CTX); > + NULL, file, pgoff, NULL, NULL_VM_UFFD_CTX, NULL); > if (vma) > goto out; > > @@ -1819,7 +1825,7 @@ unsigned long mmap_region(struct file *file, unsigned long addr, > */ > if (unlikely(vm_flags != vma->vm_flags && prev)) { > merge = vma_merge(mm, prev, vma->vm_start, vma->vm_end, vma->vm_flags, > - NULL, vma->vm_file, vma->vm_pgoff, NULL, NULL_VM_UFFD_CTX); > + NULL, vma->vm_file, vma->vm_pgoff, NULL, NULL_VM_UFFD_CTX, NULL); > if (merge) { > /* ->mmap() can change vma->vm_file and fput the original file. So > * fput the vma->vm_file here or we would add an extra fput for file > @@ -3081,7 +3087,7 @@ static int do_brk_flags(unsigned long addr, unsigned long len, unsigned long fla > > /* Can we just expand an old private anonymous mapping? */ > vma = vma_merge(mm, prev, addr, addr + len, flags, > - NULL, NULL, pgoff, NULL, NULL_VM_UFFD_CTX); > + NULL, NULL, pgoff, NULL, NULL_VM_UFFD_CTX, NULL); > if (vma) > goto out; > > @@ -3274,7 +3280,7 @@ struct vm_area_struct *copy_vma(struct vm_area_struct **vmap, > return NULL; /* should never get here */ > new_vma = vma_merge(mm, prev, addr, addr + len, vma->vm_flags, > vma->anon_vma, vma->vm_file, pgoff, vma_policy(vma), > - vma->vm_userfaultfd_ctx); > + vma->vm_userfaultfd_ctx, vma_anon_name(vma)); > if (new_vma) { > /* > * Source vma may have been merged into new_vma > diff --git a/mm/mprotect.c b/mm/mprotect.c > index 883e2cc85cad..a48ff8e79f48 100644 > --- a/mm/mprotect.c > +++ b/mm/mprotect.c > @@ -464,7 +464,7 @@ mprotect_fixup(struct vm_area_struct *vma, struct vm_area_struct **pprev, > pgoff = vma->vm_pgoff + ((start - vma->vm_start) >> PAGE_SHIFT); > *pprev = vma_merge(mm, *pprev, start, end, newflags, > vma->anon_vma, vma->vm_file, pgoff, vma_policy(vma), > - vma->vm_userfaultfd_ctx); > + vma->vm_userfaultfd_ctx, vma_anon_name(vma)); > if (*pprev) { > vma = *pprev; > VM_WARN_ON((vma->vm_flags ^ newflags) & ~VM_SOFTDIRTY); > -- > 2.33.0.153.gba50c8fa24-goog > Cool. With my notes above addressed, please consider this: Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx> -- Kees Cook
