mm: Account anon mappings as RLIMIT_DATA

When inspecting a vague code inside prctl(PR_SET_MM_MEM) call (which testing the RLIMIT_DATA value to figure out if we're allowed to assign new @start_brk, @brk, @start_data, @end_data from mm_struct) it's been commited that RLIMIT_DATA in a form it's implemented now doesn't do anything useful because most of user-space libraries use mmap() syscall for dynamic memory allocations.

So Linus suggested to convert RLIMIT_DATA rlimit into something suitable for anonymous memory accounting. Here we introduce new @anon_vm member into mm descriptor which is updated on every vm_stat_account() call.

Tests for RLIMIT_DATA are done in three places:
- mmap_region: when user calls mmap() helper
- do_brk: for brk() helper
- vma_to_resize: when mremap() is used

The do_brk() is also used in vm_brk() which is called when the kernel loads Elf and aout files to execute.

Because test for limit is done in do_brk helper we no longer need to call check_data_rlimit here.

v2:
- update doc
- add may_expand_anon_vm helper
- call for RLIMIT_DATA test in mremap and do_brk

CC Quentin Casasnovas <[email protected]>

CC Vegard Nossum <[email protected]>

CC Linus Torvalds <[email protected]>

CC Willy Tarreau <[email protected]>

CC Andy Lutomirski <[email protected]>

CC Kees Cook <[email protected]>

CC Vladimir Davydov <[email protected]>

CC Konstantin Khlebnikov <[email protected]>

CC Pavel Emelyanov <[email protected]>

CC Vladimir Davydov <[email protected]>

CC Peter Zijlstra <[email protected]>

Signed-off-by Cyrill Gorcunov <[email protected]>

---
 Documentation/filesystems/proc.txt |    1
 fs/proc/task_mmu.c                 |    2 +
 include/linux/mm_types.h           |    1
 mm/mmap.c                          |   46 ++++++++++++++++++++++---------------
 mm/mremap.c                        |    5 ++++
 5 files changed, 37 insertions(+), 18 deletions(-)

Index: linux-ml.git/Documentation/filesystems/proc.txt
===================================================================

Documentation/filesystems/proc.txt +1 -0

@@ -233,6 +233,7 @@ Table 1-2: Contents of the status files
  VmHWM                       peak resident set size ("high water mark")
  VmRSS                       size of memory portions
  VmData                      size of data, stack, and text segments
+ VmAnon                      size of anonymous private memory (not file backended)
  VmStk                       size of data, stack, and text segments
  VmExe                       size of text segment
  VmLib                       size of shared library code

Index: linux-ml.git/fs/proc/task_mmu.c
===================================================================

fs/proc/task_mmu.c +2 -0

@@ -53,6 +53,7 @@ void task_mem(struct seq_file *m, struct
                "VmHWM:\t%8lu kB\n"
                "VmRSS:\t%8lu kB\n"
                "VmData:\t%8lu kB\n"
+               "VmAnon:\t%8lu kB\n"
                "VmStk:\t%8lu kB\n"
                "VmExe:\t%8lu kB\n"
                "VmLib:\t%8lu kB\n"

@@ -66,6 +67,7 @@ void task_mem(struct seq_file *m, struct
                hiwater_rss << (PAGE_SHIFT-10),
                total_rss << (PAGE_SHIFT-10),
                data << (PAGE_SHIFT-10),
+               mm->anon_vm << (PAGE_SHIFT-10),
                mm->stack_vm << (PAGE_SHIFT-10), text, lib,
                ptes >> 10,
                pmds >> 10,

Index: linux-ml.git/include/linux/mm_types.h
===================================================================

include/linux/mm_types.h +1 -0

@@ -429,6 +429,7 @@ struct mm_struct {
        unsigned long shared_vm;        /* Shared pages (files) */
        unsigned long exec_vm;          /* VM_EXEC & ~VM_WRITE */
        unsigned long stack_vm;         /* VM_GROWSUP/DOWN */
+       unsigned long anon_vm;          /* Anonymous pages mapped */
        unsigned long def_flags;
        unsigned long start_code, end_code, start_data, end_data;
        unsigned long start_brk, brk, start_stack;

Index: linux-ml.git/mm/mmap.c
===================================================================

mm/mmap.c +28 -18

@@ -309,16 +309,6 @@ SYSCALL_DEFINE1(brk, unsigned long, brk)
        if (brk < min_brk)
                goto out;
 
-       /*
-        * Check against rlimit here. If this check is done later after the test
-        * of oldbrk with newbrk then it can escape the test and let the data
-        * segment grow beyond its set limit the in case where the limit is
-        * not page aligned -Ram Gupta
-        */
-       if (check_data_rlimit(rlimit(RLIMIT_DATA), brk, mm->start_brk,
-                             mm->end_data, mm->start_data))
-               goto out;
-
        newbrk = PAGE_ALIGN(brk);
        oldbrk = PAGE_ALIGN(mm->brk);
        if (oldbrk == newbrk)

@@ -1223,6 +1213,9 @@ void vm_stat_account(struct mm_struct *m
                        mm->exec_vm += pages;
        } else if (flags & stack_flags)
                mm->stack_vm += pages;
+
+       if (anon_accountable_mapping(file, flags))
+               mm->anon_vm += pages;
 }
 #endif /* CONFIG_PROC_FS */
 

@@ -1578,6 +1571,14 @@ unsigned long mmap_region(struct file *f
        }
 
        /*
+        * For anon mappings make sure we don't exceed the limit.
+        */
+       if (anon_accountable_mapping(file, vm_flags)) {
+               if (!may_expand_anon_vm(mm, len >> PAGE_SHIFT))
+                       return -ENOMEM;
+       }
+
+       /*
         * Can we just expand an old mapping?
         */
        vma = vma_merge(mm, prev, addr, addr + len, vm_flags,

@@ -2760,7 +2761,8 @@ static unsigned long do_brk(unsigned lon
        }
 
        /* Check against address space limits *after* clearing old maps... */
-       if (!may_expand_vm(mm, len >> PAGE_SHIFT))
+       if (!may_expand_vm(mm, len >> PAGE_SHIFT) ||
+           !may_expand_anon_vm(mm, len >> PAGE_SHIFT))
                return -ENOMEM;
 
        if (mm->map_count > sysctl_max_map_count)

@@ -2795,6 +2797,7 @@ static unsigned long do_brk(unsigned lon
 out:
        perf_event_mmap(vma);
        mm->total_vm += len >> PAGE_SHIFT;
+       mm->anon_vm += len >> PAGE_SHIFT;
        if (flags & VM_LOCKED)
                mm->locked_vm += (len >> PAGE_SHIFT);
        vma->vm_flags |= VM_SOFTDIRTY;

@@ -2982,20 +2985,27 @@ out:
        return NULL;
 }
 
+static inline int __may_expand_vm(unsigned int limit,
+                                 unsigned long cur,
+                                 unsigned long npages)
+{
+       unsigned long lim = rlimit(limit) >> PAGE_SHIFT;
+
+       return ((cur + npages) > lim) ? 0 : 1;
+}
+
 /*
  * Return true if the calling process may expand its vm space by the passed
  * number of pages
  */
 int may_expand_vm(struct mm_struct *mm, unsigned long npages)
 {
-       unsigned long cur = mm->total_vm;       /* pages */
-       unsigned long lim;
-
-       lim = rlimit(RLIMIT_AS) >> PAGE_SHIFT;
+       return __may_expand_vm(RLIMIT_AS, mm->total_vm, npages);
+}
 
-       if (cur + npages > lim)
-               return 0;
-       return 1;
+int may_expand_anon_vm(struct mm_struct *mm, unsigned long npages)
+{
+       return __may_expand_vm(RLIMIT_DATA, mm->anon_vm, npages);
 }
 
 static int special_mapping_fault(struct vm_area_struct *vma,

Index: linux-ml.git/mm/mremap.c
===================================================================

mm/mremap.c +0 -0

@@ -382,6 +382,11 @@ static struct vm_area_struct *vma_to_res
        if (!may_expand_vm(mm, (new_len - old_len) >> PAGE_SHIFT))

Here several rated changes bundled together:
* keep vma counting if CONFIG_PROC_FS=n, will be used for limits
* replace mm->shared_vm with better defined mm->data_vm
* account anonymous executable areas as executable
* account file-backed growsdown/up areas as stack
* drop struct file* argument from vm_stat_account
* enforce RLIMIT_DATA for size of data areas

This way code looks cleaner: now code/stack/data
classification depends only on vm_flags state:

VM_EXEC & ~VM_WRITE -> code (VmExe + VmLib in proc) VM_GROWSUP | VM_GROWSDOWN -> stack (VmStk) VM_WRITE & ~VM_SHARED & !stack -> data (VmData)

The rest (VmSize - VmData - VmStk - VmExe - VmLib) could be called "shared", but that might be strange beasts like readonly-private or VM_IO areas.

RLIMIT_AS limits whole address space "VmSize" RLIMIT_STACK limits stack "VmStk" (but each vma individually) RLIMIT_DATA now limits "VmData"

Signed-off-by Konstantin Khlebnikov <[email protected]>

When inspecting a vague code inside prctl(PR_SET_MM_MEM) call (which testing the RLIMIT_DATA value to figure out if we're allowed to assign new @start_brk, @brk, @start_data, @end_data from mm_struct) it's been commited that RLIMIT_DATA in a form it's implemented now doesn't do anything useful because most of user-space libraries use mmap() syscall for dynamic memory allocations.

So Linus suggested to convert RLIMIT_DATA rlimit into something suitable for anonymous memory accounting. Here we introduce new @anon_vm member into mm descriptor which is updated on every vm_stat_account() call.

Tests for RLIMIT_DATA are done in three places:
- mmap_region: when user calls mmap() helper
- do_brk: for brk() helper
- vma_to_resize: when mremap() is used

The do_brk() is also used in vm_brk() which is called when the kernel loads Elf and aout files to execute.

Because test for limit is done in do_brk helper we no longer need to call check_data_rlimit here.

v2:
- update doc
- add may_expand_anon_vm helper
- call for RLIMIT_DATA test in mremap and do_brk

CC Quentin Casasnovas <[email protected]>

CC Vegard Nossum <[email protected]>

CC Linus Torvalds <[email protected]>

CC Willy Tarreau <[email protected]>

CC Andy Lutomirski <[email protected]>

CC Kees Cook <[email protected]>

CC Vladimir Davydov <[email protected]>

CC Konstantin Khlebnikov <[email protected]>

CC Pavel Emelyanov <[email protected]>

CC Vladimir Davydov <[email protected]>

CC Peter Zijlstra <[email protected]>

Signed-off-by Cyrill Gorcunov <[email protected]>

---
 Documentation/filesystems/proc.txt |    1 
 fs/proc/task_mmu.c                 |    2 +
 include/linux/mm_types.h           |    1 
 mm/mmap.c                          |   46 ++++++++++++++++++++++---------------
 mm/mremap.c                        |    5 ++++
 5 files changed, 37 insertions(+), 18 deletions(-)

Index: linux-ml.git/Documentation/filesystems/proc.txt
===================================================================

Documentation/filesystems/proc.txt +1 -0

@@ -233,6 +233,7 @@ Table 1-2: Contents of the status files
  VmHWM                       peak resident set size ("high water mark")
  VmRSS                       size of memory portions
  VmData                      size of data, stack, and text segments
+ VmAnon                      size of anonymous private memory (not file backended)
  VmStk                       size of data, stack, and text segments
  VmExe                       size of text segment
  VmLib                       size of shared library code

Index: linux-ml.git/fs/proc/task_mmu.c
===================================================================

fs/proc/task_mmu.c +2 -0

@@ -53,6 +53,7 @@ void task_mem(struct seq_file *m, struct
 		"VmHWM:\t%8lu kB\n"
 		"VmRSS:\t%8lu kB\n"
 		"VmData:\t%8lu kB\n"
+		"VmAnon:\t%8lu kB\n"
 		"VmStk:\t%8lu kB\n"
 		"VmExe:\t%8lu kB\n"
 		"VmLib:\t%8lu kB\n"

@@ -66,6 +67,7 @@ void task_mem(struct seq_file *m, struct
 		hiwater_rss << (PAGE_SHIFT-10),
 		total_rss << (PAGE_SHIFT-10),
 		data << (PAGE_SHIFT-10),
+		mm->anon_vm << (PAGE_SHIFT-10),
 		mm->stack_vm << (PAGE_SHIFT-10), text, lib,
 		ptes >> 10,
 		pmds >> 10,

Index: linux-ml.git/include/linux/mm_types.h
===================================================================

include/linux/mm_types.h +1 -0

@@ -429,6 +429,7 @@ struct mm_struct {
 	unsigned long shared_vm;	/* Shared pages (files) */
 	unsigned long exec_vm;		/* VM_EXEC & ~VM_WRITE */
 	unsigned long stack_vm;		/* VM_GROWSUP/DOWN */
+	unsigned long anon_vm;		/* Anonymous pages mapped */
 	unsigned long def_flags;
 	unsigned long start_code, end_code, start_data, end_data;
 	unsigned long start_brk, brk, start_stack;

Index: linux-ml.git/mm/mmap.c
===================================================================

mm/mmap.c +24 -15

@@ -309,16 +309,6 @@ SYSCALL_DEFINE1(brk, unsigned long, brk)
 	if (brk < min_brk)
 		goto out;
 
-	/*
-	 * Check against rlimit here. If this check is done later after the test
-	 * of oldbrk with newbrk then it can escape the test and let the data
-	 * segment grow beyond its set limit the in case where the limit is
-	 * not page aligned -Ram Gupta
-	 */
-	if (check_data_rlimit(rlimit(RLIMIT_DATA), brk, mm->start_brk,
-			      mm->end_data, mm->start_data))
-		goto out;
-
 	newbrk = PAGE_ALIGN(brk);
 	oldbrk = PAGE_ALIGN(mm->brk);
 	if (oldbrk == newbrk)

@@ -1223,6 +1213,9 @@ void vm_stat_account(struct mm_struct *m
 			mm->exec_vm += pages;
 	} else if (flags & stack_flags)
 		mm->stack_vm += pages;
+
+	if (anon_accountable_mapping(file, flags))
+		mm->anon_vm += pages;
 }
 #endif /* CONFIG_PROC_FS */
 

@@ -1578,6 +1571,14 @@ unsigned long mmap_region(struct file *f
 	}
 
 	/*
+	 * For anon mappings make sure we don't exceed the limit.
+	 */
+	if (anon_accountable_mapping(file, vm_flags)) {
+		if (!may_expand_anon_vm(mm, len >> PAGE_SHIFT))
+			return -ENOMEM;
+	}
+
+	/*
 	 * Can we just expand an old mapping?
 	 */
 	vma = vma_merge(mm, prev, addr, addr + len, vm_flags,

@@ -2760,7 +2761,8 @@ static unsigned long do_brk(unsigned lon
 	}
 
 	/* Check against address space limits *after* clearing old maps... */
-	if (!may_expand_vm(mm, len >> PAGE_SHIFT))
+	if (!may_expand_vm(mm, len >> PAGE_SHIFT) ||
+	    !may_expand_anon_vm(mm, len >> PAGE_SHIFT))
 		return -ENOMEM;
 
 	if (mm->map_count > sysctl_max_map_count)

@@ -2795,6 +2797,7 @@ static unsigned long do_brk(unsigned lon
 out:
 	perf_event_mmap(vma);
 	mm->total_vm += len >> PAGE_SHIFT;
+	mm->anon_vm += len >> PAGE_SHIFT;
 	if (flags & VM_LOCKED)
 		mm->locked_vm += (len >> PAGE_SHIFT);
 	vma->vm_flags |= VM_SOFTDIRTY;

@@ -2982,20 +2985,27 @@ out:
 	return NULL;
 }
 
+static inline int __may_expand_vm(unsigned int limit,
+				  unsigned long cur,
+				  unsigned long npages)
+{
+	unsigned long lim = rlimit(limit) >> PAGE_SHIFT;
+
+	return ((cur + npages) > lim) ? 0 : 1;
+}
+
 /*
  * Return true if the calling process may expand its vm space by the passed
  * number of pages
  */
 int may_expand_vm(struct mm_struct *mm, unsigned long npages)
 {
-	unsigned long cur = mm->total_vm;	/* pages */
-	unsigned long lim;
-
-	lim = rlimit(RLIMIT_AS) >> PAGE_SHIFT;
+	return __may_expand_vm(RLIMIT_AS, mm->total_vm, npages);

Do we want to fold may_expand_anon_vm() into may_expand_vm() (potentially passing it the flags/struct file if needed) so there is just one such helper function? Rationale being that it then gets hard to see what restricts what, and it's easy to miss one place.

For example, I couldn't find anything preventing a user to mmap(MAP_GROWSDOWN) and uses that as a base to get pages that would not be accounted for in your patch (making it a poor-man mremap()).

I only had a quick look so apologies if this is handled and I missed it :)

On Mon, Dec 14, 2015 at 03:51:26PM +0100, Quentin Casasnovas wrote:
...

Quoted 5 lines

Do we want to fold may_expand_anon_vm() into may_expand_vm() (potentially passing it the flags/struct file if needed) so there is just one such helper function? Rationale being that it then gets hard to see what restricts what, and it's easy to miss one place.

I tried to make the patch small as possible (because otherwise indeed I would have to pass @vm_file|@file as additional argument). This won't be a problem but may_expand_vm is called way more times than may_expand_anon_vm. That's the only rationale I followed.

Quoted 3 lines

For example, I couldn't find anything preventing a user to mmap(MAP_GROWSDOWN) and uses that as a base to get pages that would not be accounted for in your patch (making it a poor-man mremap()).

growsup/down stand for stack usage iirc, so it was intentionally not accounted here.

Quoted 2 lines

I only had a quick look so apologies if this is handled and I missed it :)

thanks for feedback! also take a look on Kostya's patch, I think it's even better approach (and I like it more than mine).

Quoted 4 lines

growsup/down stand for stack usage iirc, so it was intentionally not accounted here.

Right, but in the same vein of Linus saying RLIMIT_DATA is/was useless because everyone could use mmap() instead of brk() to get anonymous memory, what's the point of restricting "almost-all" anonymous memory if one can just use MAP_GROWSDOWN/UP and cause repeated page faults to extend that mapping, circumventing your checks? That makes the new restriction as useless as what RLIMIT_DATA used to be, doesn't it?

Quoted 5 lines

Quoted 2 lines

I only had a quick look so apologies if this is handled and I missed it :)

thanks for feedback! also take a look on Kostya's patch, I think it's even better approach (and I like it more than mine).

Ha I'm not subscribed to LKML so I missed those, I suppose you can ignore my comments then! :)