rlimit_as-checking-fix.patch

rlimit_as-checking-fix.patch

[Andrew Morton]

review, please?



Address bug #4508: there's potential for wraparound in the various places
where we perform RLIMIT_AS checking.

(I'm a bit worried about acct_stack_growth().  Are we sure that vma->vm_mm is
always equal to current->mm?  If not, then we're comapring some other
process's total_vm with the calling process's rlimits).


Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 include/linux/mm.h |    1 +
 mm/mmap.c          |   24 +++++++++++++++++++-----
 mm/mremap.c        |    6 +++---
 3 files changed, 23 insertions(+), 8 deletions(-)

diff -puN mm/mmap.c~rlimit_as-checking-fix mm/mmap.c
--- 25/mm/mmap.c~rlimit_as-checking-fix	2005-04-25 19:54:35.685820728 -0700
+++ 25-akpm/mm/mmap.c	2005-04-25 19:54:41.867880912 -0700
@@ -1009,8 +1009,7 @@ munmap_back:
 	}
 
 	/* Check against address space limit. */
-	if ((mm->total_vm << PAGE_SHIFT) + len
-	    > current->signal->rlim[RLIMIT_AS].rlim_cur)
+	if (!may_expand_vm(mm, len))
 		return -ENOMEM;
 
 	if (accountable && (!(flags & MAP_NORESERVE) ||
@@ -1421,7 +1420,7 @@ static int acct_stack_growth(struct vm_a
 	struct rlimit *rlim = current->signal->rlim;
 
 	/* address space limit tests */
-	if (mm->total_vm + grow > rlim[RLIMIT_AS].rlim_cur >> PAGE_SHIFT)
+	if (!may_expand_vm(mm, grow))
 		return -ENOMEM;
 
 	/* Stack limit test */
@@ -1848,8 +1847,7 @@ unsigned long do_brk(unsigned long addr,
 	}
 
 	/* Check against address space limits *after* clearing old maps... */
-	if ((mm->total_vm << PAGE_SHIFT) + len
-	    > current->signal->rlim[RLIMIT_AS].rlim_cur)
+	if (!may_expand_vm(mm, len))
 		return -ENOMEM;
 
 	if (mm->map_count > sysctl_max_map_count)
@@ -2019,3 +2017,19 @@ struct vm_area_struct *copy_vma(struct v
 	}
 	return new_vma;
 }
+
+/*
+ * Return true if the calling process may expand its vm space by the passed
+ * number of bytes.
+ */
+int may_expand_vm(struct mm_struct *mm, unsigned long nbytes)
+{
+	unsigned long cur = current->mm->total_vm;	/* pages */
+	unsigned long lim;
+
+	lim = current->signal->rlim[RLIMIT_AS].rlim_cur >> PAGE_SHIFT;
+
+	if (cur + nbytes > lim)
+		return 0;
+	return 1;
+}
diff -puN mm/mremap.c~rlimit_as-checking-fix mm/mremap.c
--- 25/mm/mremap.c~rlimit_as-checking-fix	2005-04-25 19:54:35.685820728 -0700
+++ 25-akpm/mm/mremap.c	2005-04-25 19:54:41.868880760 -0700
@@ -347,10 +347,10 @@ unsigned long do_mremap(unsigned long ad
 		if (locked > lock_limit && !capable(CAP_IPC_LOCK))
 			goto out;
 	}
-	ret = -ENOMEM;
-	if ((current->mm->total_vm << PAGE_SHIFT) + (new_len - old_len)
-	    > current->signal->rlim[RLIMIT_AS].rlim_cur)
+	if (!may_expand_vm(current->mm, new_len - old_len)) {
+		ret = -ENOMEM;
 		goto out;
+	}
 
 	if (vma->vm_flags & VM_ACCOUNT) {
 		charged = (new_len - old_len) >> PAGE_SHIFT;
diff -puN include/linux/mm.h~rlimit_as-checking-fix include/linux/mm.h
--- 25/include/linux/mm.h~rlimit_as-checking-fix	2005-04-25 19:54:35.686820576 -0700
+++ 25-akpm/include/linux/mm.h	2005-04-25 19:54:41.869880608 -0700
@@ -726,6 +726,7 @@ extern void __vma_link_rb(struct mm_stru
 extern struct vm_area_struct *copy_vma(struct vm_area_struct **,
 	unsigned long addr, unsigned long len, pgoff_t pgoff);
 extern void exit_mmap(struct mm_struct *);
+extern int may_expand_vm(struct mm_struct *mm, unsigned long nbytes);
 
 extern unsigned long get_unmapped_area(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
 
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: rlimit_as-checking-fix.patch

[Andrew Morton]

Andrew Morton <akpm@osdl.org> wrote:
>
> review, please?

crappy reviewers.  Tested version.




Address bug #4508: there's potential for wraparound in the various places
where we perform RLIMIT_AS checking.

(I'm a bit worried about acct_stack_growth().  Are we sure that vma->vm_mm is
always equal to current->mm?  If not, then we're comparing some other
process's total_vm with the calling process's rlimits).


Signed-off-by: Andrew Morton <akpm@osdl.org>
---

 include/linux/mm.h |    1 +
 mm/mmap.c          |   24 +++++++++++++++++++-----
 mm/mremap.c        |    6 +++---
 3 files changed, 23 insertions(+), 8 deletions(-)

diff -puN mm/mmap.c~rlimit_as-checking-fix mm/mmap.c
--- 25/mm/mmap.c~rlimit_as-checking-fix	2005-04-26 22:40:12.316426776 -0700
+++ 25-akpm/mm/mmap.c	2005-04-26 22:56:03.548817472 -0700
@@ -1009,8 +1009,7 @@ munmap_back:
 	}
 
 	/* Check against address space limit. */
-	if ((mm->total_vm << PAGE_SHIFT) + len
-	    > current->signal->rlim[RLIMIT_AS].rlim_cur)
+	if (!may_expand_vm(mm, len >> PAGE_SHIFT))
 		return -ENOMEM;
 
 	if (accountable && (!(flags & MAP_NORESERVE) ||
@@ -1421,7 +1420,7 @@ static int acct_stack_growth(struct vm_a
 	struct rlimit *rlim = current->signal->rlim;
 
 	/* address space limit tests */
-	if (mm->total_vm + grow > rlim[RLIMIT_AS].rlim_cur >> PAGE_SHIFT)
+	if (!may_expand_vm(mm, grow))
 		return -ENOMEM;
 
 	/* Stack limit test */
@@ -1848,8 +1847,7 @@ unsigned long do_brk(unsigned long addr,
 	}
 
 	/* Check against address space limits *after* clearing old maps... */
-	if ((mm->total_vm << PAGE_SHIFT) + len
-	    > current->signal->rlim[RLIMIT_AS].rlim_cur)
+	if (!may_expand_vm(mm, len >> PAGE_SHIFT))
 		return -ENOMEM;
 
 	if (mm->map_count > sysctl_max_map_count)
@@ -2019,3 +2017,19 @@ struct vm_area_struct *copy_vma(struct v
 	}
 	return new_vma;
 }
+
+/*
+ * 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 = current->mm->total_vm;	/* pages */
+	unsigned long lim;
+
+	lim = current->signal->rlim[RLIMIT_AS].rlim_cur >> PAGE_SHIFT;
+
+	if (cur + npages > lim)
+		return 0;
+	return 1;
+}
diff -puN mm/mremap.c~rlimit_as-checking-fix mm/mremap.c
--- 25/mm/mremap.c~rlimit_as-checking-fix	2005-04-26 22:40:12.318426472 -0700
+++ 25-akpm/mm/mremap.c	2005-04-26 22:52:30.489207456 -0700
@@ -347,10 +347,10 @@ unsigned long do_mremap(unsigned long ad
 		if (locked > lock_limit && !capable(CAP_IPC_LOCK))
 			goto out;
 	}
-	ret = -ENOMEM;
-	if ((current->mm->total_vm << PAGE_SHIFT) + (new_len - old_len)
-	    > current->signal->rlim[RLIMIT_AS].rlim_cur)
+	if (!may_expand_vm(current->mm, (new_len - old_len) >> PAGE_SHIFT)) {
+		ret = -ENOMEM;
 		goto out;
+	}
 
 	if (vma->vm_flags & VM_ACCOUNT) {
 		charged = (new_len - old_len) >> PAGE_SHIFT;
diff -puN include/linux/mm.h~rlimit_as-checking-fix include/linux/mm.h
--- 25/include/linux/mm.h~rlimit_as-checking-fix	2005-04-26 22:40:12.319426320 -0700
+++ 25-akpm/include/linux/mm.h	2005-04-26 22:52:34.744560544 -0700
@@ -761,6 +761,7 @@ extern void __vma_link_rb(struct mm_stru
 extern struct vm_area_struct *copy_vma(struct vm_area_struct **,
 	unsigned long addr, unsigned long len, pgoff_t pgoff);
 extern void exit_mmap(struct mm_struct *);
+extern int may_expand_vm(struct mm_struct *mm, unsigned long npages);
 
 extern unsigned long get_unmapped_area(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
 
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: rlimit_as-checking-fix.patch

[Chris Wright]

* Andrew Morton (akpm@osdl.org) wrote:
> Andrew Morton <akpm@osdl.org> wrote:
> >
> > review, please?
> 
> crappy reviewers.  Tested version.

Sorry to be so slow on that one.  Here's an update for the mlock part.
It's only compile tested as of yet.




Always use page counts when doing RLIMIT_MEMLOCK checking to avoid
possible overflow.

Signed-off-by: Chris Wright <chrisw@osdl.org>
---

mm/mmap.c: 6ea204cc751e4d2f0fe4a7d213bab9ae90ad58c4
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -937,9 +937,10 @@ unsigned long do_mmap_pgoff(struct file 
 	/* mlock MCL_FUTURE? */
 	if (vm_flags & VM_LOCKED) {
 		unsigned long locked, lock_limit;
-		locked = mm->locked_vm << PAGE_SHIFT;
+		locked = len >> PAGE_SHIFT;
+		locked += mm->locked_vm;
 		lock_limit = current->signal->rlim[RLIMIT_MEMLOCK].rlim_cur;
-		locked += len;
+		lock_limit >>= PAGE_SHIFT;
 		if (locked > lock_limit && !capable(CAP_IPC_LOCK))
 			return -EAGAIN;
 	}
@@ -1823,9 +1824,10 @@ unsigned long do_brk(unsigned long addr,
 	 */
 	if (mm->def_flags & VM_LOCKED) {
 		unsigned long locked, lock_limit;
-		locked = mm->locked_vm << PAGE_SHIFT;
+		locked = len >> PAGE_SHIFT;
+		locked += mm->locked_vm;
 		lock_limit = current->signal->rlim[RLIMIT_MEMLOCK].rlim_cur;
-		locked += len;
+		lock_limit >>= PAGE_SHIFT;
 		if (locked > lock_limit && !capable(CAP_IPC_LOCK))
 			return -EAGAIN;
 	}
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: rlimit_as-checking-fix.patch

[Chris Wright]

* Andrew Morton (akpm@osdl.org) wrote:
> review, please?
> 
> Address bug #4508: there's potential for wraparound in the various places
> where we perform RLIMIT_AS checking.

Hrm, I suppose RLIMIT_MEMLOCK has same fundamental problem.

> (I'm a bit worried about acct_stack_growth().  Are we sure that vma->vm_mm is
> always equal to current->mm?  If not, then we're comapring some other
> process's total_vm with the calling process's rlimits).

No, get_user_pages() looks like an offender here.  Fine for all cases
but ptrace.  In some odd way, it makes sense to compare against caller rlimits
for ptrace.  Real problem would be accounting to wrong mm->total_vm.

> Signed-off-by: Andrew Morton <akpm@osdl.org>
> ---
> 
>  include/linux/mm.h |    1 +
>  mm/mmap.c          |   24 +++++++++++++++++++-----
>  mm/mremap.c        |    6 +++---
>  3 files changed, 23 insertions(+), 8 deletions(-)
> 
> diff -puN mm/mmap.c~rlimit_as-checking-fix mm/mmap.c
> --- 25/mm/mmap.c~rlimit_as-checking-fix	2005-04-25 19:54:35.685820728 -0700
> +++ 25-akpm/mm/mmap.c	2005-04-25 19:54:41.867880912 -0700
> @@ -1009,8 +1009,7 @@ munmap_back:
>  	}
>  
>  	/* Check against address space limit. */
> -	if ((mm->total_vm << PAGE_SHIFT) + len
> -	    > current->signal->rlim[RLIMIT_AS].rlim_cur)
> +	if (!may_expand_vm(mm, len))
>  		return -ENOMEM;
>  
>  	if (accountable && (!(flags & MAP_NORESERVE) ||
> @@ -1421,7 +1420,7 @@ static int acct_stack_growth(struct vm_a
>  	struct rlimit *rlim = current->signal->rlim;
>  
>  	/* address space limit tests */
> -	if (mm->total_vm + grow > rlim[RLIMIT_AS].rlim_cur >> PAGE_SHIFT)
> +	if (!may_expand_vm(mm, grow))

Here grow is already in pages.

>  		return -ENOMEM;
>  
>  	/* Stack limit test */
> @@ -1848,8 +1847,7 @@ unsigned long do_brk(unsigned long addr,
>  	}
>  
>  	/* Check against address space limits *after* clearing old maps... */
> -	if ((mm->total_vm << PAGE_SHIFT) + len
> -	    > current->signal->rlim[RLIMIT_AS].rlim_cur)
> +	if (!may_expand_vm(mm, len))
>  		return -ENOMEM;
>  
>  	if (mm->map_count > sysctl_max_map_count)
> @@ -2019,3 +2017,19 @@ struct vm_area_struct *copy_vma(struct v
>  	}
>  	return new_vma;
>  }
> +
> +/*
> + * Return true if the calling process may expand its vm space by the passed
> + * number of bytes.
> + */
> +int may_expand_vm(struct mm_struct *mm, unsigned long nbytes)
> +{
> +	unsigned long cur = current->mm->total_vm;	/* pages */

I guess you meant simply mm->total_vm not current->mm->total_vm?

> +	unsigned long lim;
> +
> +	lim = current->signal->rlim[RLIMIT_AS].rlim_cur >> PAGE_SHIFT;
> +
> +	if (cur + nbytes > lim)

I think there's a missing shift on nbytes to become a page count as well?

> +		return 0;
> +	return 1;
> +}
> diff -puN mm/mremap.c~rlimit_as-checking-fix mm/mremap.c
> --- 25/mm/mremap.c~rlimit_as-checking-fix	2005-04-25 19:54:35.685820728 -0700
> +++ 25-akpm/mm/mremap.c	2005-04-25 19:54:41.868880760 -0700
> @@ -347,10 +347,10 @@ unsigned long do_mremap(unsigned long ad
>  		if (locked > lock_limit && !capable(CAP_IPC_LOCK))
>  			goto out;
>  	}
> -	ret = -ENOMEM;
> -	if ((current->mm->total_vm << PAGE_SHIFT) + (new_len - old_len)
> -	    > current->signal->rlim[RLIMIT_AS].rlim_cur)
> +	if (!may_expand_vm(current->mm, new_len - old_len)) {
> +		ret = -ENOMEM;
>  		goto out;
> +	}
>  
>  	if (vma->vm_flags & VM_ACCOUNT) {
>  		charged = (new_len - old_len) >> PAGE_SHIFT;
> diff -puN include/linux/mm.h~rlimit_as-checking-fix include/linux/mm.h
> --- 25/include/linux/mm.h~rlimit_as-checking-fix	2005-04-25 19:54:35.686820576 -0700
> +++ 25-akpm/include/linux/mm.h	2005-04-25 19:54:41.869880608 -0700
> @@ -726,6 +726,7 @@ extern void __vma_link_rb(struct mm_stru
>  extern struct vm_area_struct *copy_vma(struct vm_area_struct **,
>  	unsigned long addr, unsigned long len, pgoff_t pgoff);
>  extern void exit_mmap(struct mm_struct *);
> +extern int may_expand_vm(struct mm_struct *mm, unsigned long nbytes);
>  
>  extern unsigned long get_unmapped_area(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
>  
> _

-- 
Linux Security Modules     http://lsm.immunix.org     http://lsm.bkbits.net
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>

Re: rlimit_as-checking-fix.patch

[Andrew Morton]

Chris Wright <chrisw@osdl.org> wrote:
>
> > +int may_expand_vm(struct mm_struct *mm, unsigned long nbytes)
>  > +{
>  > +	unsigned long cur = current->mm->total_vm;	/* pages */
> 
>  I guess you meant simply mm->total_vm not current->mm->total_vm?

Yes.  Sigh.

diff -puN mm/mmap.c~rlimit_as-checking-fix mm/mmap.c
--- 25/mm/mmap.c~rlimit_as-checking-fix	2005-04-26 22:40:12.316426776 -0700
+++ 25-akpm/mm/mmap.c	2005-04-26 23:27:26.549557848 -0700
@@ -1009,8 +1009,7 @@ munmap_back:
 	}
 
 	/* Check against address space limit. */
-	if ((mm->total_vm << PAGE_SHIFT) + len
-	    > current->signal->rlim[RLIMIT_AS].rlim_cur)
+	if (!may_expand_vm(mm, len >> PAGE_SHIFT))
 		return -ENOMEM;
 
 	if (accountable && (!(flags & MAP_NORESERVE) ||
@@ -1421,7 +1420,7 @@ static int acct_stack_growth(struct vm_a
 	struct rlimit *rlim = current->signal->rlim;
 
 	/* address space limit tests */
-	if (mm->total_vm + grow > rlim[RLIMIT_AS].rlim_cur >> PAGE_SHIFT)
+	if (!may_expand_vm(mm, grow))
 		return -ENOMEM;
 
 	/* Stack limit test */
@@ -1848,8 +1847,7 @@ unsigned long do_brk(unsigned long addr,
 	}
 
 	/* Check against address space limits *after* clearing old maps... */
-	if ((mm->total_vm << PAGE_SHIFT) + len
-	    > current->signal->rlim[RLIMIT_AS].rlim_cur)
+	if (!may_expand_vm(mm, len >> PAGE_SHIFT))
 		return -ENOMEM;
 
 	if (mm->map_count > sysctl_max_map_count)
@@ -2019,3 +2017,19 @@ struct vm_area_struct *copy_vma(struct v
 	}
 	return new_vma;
 }
+
+/*
+ * 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 = current->signal->rlim[RLIMIT_AS].rlim_cur >> PAGE_SHIFT;
+
+	if (cur + npages > lim)
+		return 0;
+	return 1;
+}
diff -puN mm/mremap.c~rlimit_as-checking-fix mm/mremap.c
--- 25/mm/mremap.c~rlimit_as-checking-fix	2005-04-26 22:40:12.318426472 -0700
+++ 25-akpm/mm/mremap.c	2005-04-26 22:52:30.489207456 -0700
@@ -347,10 +347,10 @@ unsigned long do_mremap(unsigned long ad
 		if (locked > lock_limit && !capable(CAP_IPC_LOCK))
 			goto out;
 	}
-	ret = -ENOMEM;
-	if ((current->mm->total_vm << PAGE_SHIFT) + (new_len - old_len)
-	    > current->signal->rlim[RLIMIT_AS].rlim_cur)
+	if (!may_expand_vm(current->mm, (new_len - old_len) >> PAGE_SHIFT)) {
+		ret = -ENOMEM;
 		goto out;
+	}
 
 	if (vma->vm_flags & VM_ACCOUNT) {
 		charged = (new_len - old_len) >> PAGE_SHIFT;
diff -puN include/linux/mm.h~rlimit_as-checking-fix include/linux/mm.h
--- 25/include/linux/mm.h~rlimit_as-checking-fix	2005-04-26 22:40:12.000000000 -0700
+++ 25-akpm/include/linux/mm.h	2005-04-26 22:52:34.000000000 -0700
@@ -761,6 +761,7 @@ extern void __vma_link_rb(struct mm_stru
 extern struct vm_area_struct *copy_vma(struct vm_area_struct **,
 	unsigned long addr, unsigned long len, pgoff_t pgoff);
 extern void exit_mmap(struct mm_struct *);
+extern int may_expand_vm(struct mm_struct *mm, unsigned long npages);
 
 extern unsigned long get_unmapped_area(struct file *, unsigned long, unsigned long, unsigned long, unsigned long);
 
_

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"aart@kvack.org"> aart@kvack.org </a>