Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1676927 > unrolled thread

[PATCH 1/1] expand_downwards: don't require the gap if !vm_prev

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 1/1] expand_downwards: don't require the gap if !vm_prev Oleg Nesterov <oleg@redhat.com> - 2017-06-28 20:00 +0200
    Re: [PATCH 1/1] expand_downwards: don't require the gap if !vm_prev Michal Hocko <mhocko@kernel.org> - 2017-06-30 15:20 +0200

#1676927 — [PATCH 1/1] expand_downwards: don't require the gap if !vm_prev

expand_stack(vma) fails if address < stack_guard_gap even if there is no
vma->vm_prev. I don't think this makes sense, and we didn't do this before
the recent commit 1be7107fbe18 ("mm: larger stack guard gap, between vmas").
We do not need a gap in this case, any address is fine as long as
security_mmap_addr() doesn't object.

This also simplifies the code, we know that address >= prev->vm_end and
thus underflow is not possible.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
 mm/mmap.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 8e07976..5a8bd97 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2310,7 +2310,6 @@ int expand_downwards(struct vm_area_struct *vma,
 {
 	struct mm_struct *mm = vma->vm_mm;
 	struct vm_area_struct *prev;
-	unsigned long gap_addr;
 	int error;
 
 	address &= PAGE_MASK;
@@ -2319,14 +2318,11 @@ int expand_downwards(struct vm_area_struct *vma,
 		return error;
 
 	/* Enforce stack_guard_gap */
-	gap_addr = address - stack_guard_gap;
-	if (gap_addr > address)
-		return -ENOMEM;
 	prev = vma->vm_prev;
-	if (prev && prev->vm_end > gap_addr) {
-		if (!(prev->vm_flags & VM_GROWSDOWN))
+	/* Check that both stack segments have the same anon_vma? */
+	if (prev && !(prev->vm_flags & VM_GROWSDOWN)) {
+		if (address - prev->vm_end < stack_guard_gap)
 			return -ENOMEM;
-		/* Check that both stack segments have the same anon_vma? */
 	}
 
 	/* We must make sure the anon_vma is allocated. */
-- 
2.5.0

[toc] | [next] | [standalone]

#1678806

On Wed 28-06-17 19:52:58, Oleg Nesterov wrote:
> expand_stack(vma) fails if address < stack_guard_gap even if there is no
> vma->vm_prev. I don't think this makes sense, and we didn't do this before
> the recent commit 1be7107fbe18 ("mm: larger stack guard gap, between vmas").
> We do not need a gap in this case, any address is fine as long as
> security_mmap_addr() doesn't object.
> 
> This also simplifies the code, we know that address >= prev->vm_end and
> thus underflow is not possible.
> 
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>

Acked-by: Michal Hocko <mhocko@suse.com>

> ---
>  mm/mmap.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 8e07976..5a8bd97 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2310,7 +2310,6 @@ int expand_downwards(struct vm_area_struct *vma,
>  {
>  	struct mm_struct *mm = vma->vm_mm;
>  	struct vm_area_struct *prev;
> -	unsigned long gap_addr;
>  	int error;
>  
>  	address &= PAGE_MASK;
> @@ -2319,14 +2318,11 @@ int expand_downwards(struct vm_area_struct *vma,
>  		return error;
>  
>  	/* Enforce stack_guard_gap */
> -	gap_addr = address - stack_guard_gap;
> -	if (gap_addr > address)
> -		return -ENOMEM;

I thought this was an underflow protection. address might be still above
min_mmap address while gap_addr can underflow, that would mean that we
might not detect a mapping in that range, but

>  	prev = vma->vm_prev;
> -	if (prev && prev->vm_end > gap_addr) {
> -		if (!(prev->vm_flags & VM_GROWSDOWN))
> +	/* Check that both stack segments have the same anon_vma? */
> +	if (prev && !(prev->vm_flags & VM_GROWSDOWN)) {
> +		if (address - prev->vm_end < stack_guard_gap)

this would handle that case properly so the problem wouldn't happen.

>  			return -ENOMEM;
> -		/* Check that both stack segments have the same anon_vma? */
>  	}
>  
>  	/* We must make sure the anon_vma is allocated. */
> -- 
> 2.5.0
> 
> 

-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web