Re: [2/3] mm: fix up some user-visible effects of the stack guard page

[Linus Torvalds]

On Mon, Jan 5, 2015 at 2:21 AM, Jay Foad <jay.foad@xxxxxxxxx> wrote:
> Sorry for replying to this old email...
>
>> commit d7824370e26325c881b665350ce64fb0a4fde24a upstream

Heh. From august 2010. That's 4+ years ago.. How come it was noticed
only now? You guys are running excessively old kernels, methinks.

> Address sanitizer tries to find the mapping for the current thread's
> stack by iterating through the entries in /proc/self/maps looking for
> one that contains the address of some random stack variable. This
> fails if the stack mapping has already used up all of its RLIMIT_STACK
> quota, because in that case check_stack_guard_page() will fail to add
> a guard page, but show_map_vma() will still assume that the first page
> of the stack *is* a guard page, and won't report it in /proc/maps.
>
> Here's a small program that demonstrates the failure:

Yup, your analysis sounds correct.  My completely untested gut feel is
that the problem is that we don't actually return the error from the
expand_stack() call, so then do_anonymous_page() will allow the extra
guard-page access.

IOW, *maybe* a patch like this. TOTALLY UNTESTED! I may have missed
something, and this may be complete crap.

                                 Linus
 include/linux/mm.h | 2 +-
 mm/memory.c        | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index f80d0194c9bc..80fc92a49649 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1952,7 +1952,7 @@ extern int expand_downwards(struct vm_area_struct *vma,
 #if VM_GROWSUP
 extern int expand_upwards(struct vm_area_struct *vma, unsigned long address);
 #else
-  #define expand_upwards(vma, address) do { } while (0)
+  #define expand_upwards(vma, address) (0)
 #endif
 
 /* Look up the first VMA which satisfies  addr < vm_end,  NULL if none. */
diff --git a/mm/memory.c b/mm/memory.c
index ca920d1fd314..d7e497e98f46 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2593,7 +2593,7 @@ static inline int check_stack_guard_page(struct vm_area_struct *vma, unsigned lo
 		if (prev && prev->vm_end == address)
 			return prev->vm_flags & VM_GROWSDOWN ? 0 : -ENOMEM;
 
-		expand_downwards(vma, address - PAGE_SIZE);
+		return expand_downwards(vma, address - PAGE_SIZE);
 	}
 	if ((vma->vm_flags & VM_GROWSUP) && address + PAGE_SIZE == vma->vm_end) {
 		struct vm_area_struct *next = vma->vm_next;
@@ -2602,7 +2602,7 @@ static inline int check_stack_guard_page(struct vm_area_struct *vma, unsigned lo
 		if (next && next->vm_start == address + PAGE_SIZE)
 			return next->vm_flags & VM_GROWSUP ? 0 : -ENOMEM;
 
-		expand_upwards(vma, address + PAGE_SIZE);
+		return expand_upwards(vma, address + PAGE_SIZE);
 	}
 	return 0;
 }