pa-risc and ia64 have stacks that grow upwards. Check that
they do not run into other mappings.
Signed-off-by: Tony Luck <tony.luck@intel.com>
---
Updated to match the new code - still not tested on pa-risc.
The #ifdefs are ugly - suggestions welcome on how to make
the code prettier.
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 709f672..089d135 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -1330,7 +1330,7 @@ unsigned long ra_submit(struct file_ra_state *ra,
/* Do stack extension */
extern int expand_stack(struct vm_area_struct *vma, unsigned long address);
-#ifdef CONFIG_IA64
+#if defined(CONFIG_STACK_GROWSUP) || defined(CONFIG_IA64)
extern int expand_upwards(struct vm_area_struct *vma, unsigned long address);
#endif
extern int expand_stack_downwards(struct vm_area_struct *vma,
diff --git a/mm/memory.c b/mm/memory.c
index 2ed2267..5127b1c 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -2760,29 +2760,43 @@ out_release:
}
/*
- * This is like a special single-page "expand_downwards()",
- * except we must first make sure that 'address-PAGE_SIZE'
+ * This is like a special single-page "expand_{down|up}wards()",
+ * except we must first make sure that 'address{-|+}PAGE_SIZE'
* doesn't hit another vma.
- *
- * The "find_vma()" will do the right thing even if we wrap
*/
static inline int check_stack_guard_page(struct vm_area_struct *vma, unsigned long address)
{
- address &= PAGE_MASK;
- if ((vma->vm_flags & VM_GROWSDOWN) && address == vma->vm_start) {
- struct vm_area_struct *prev = vma->vm_prev;
+ if (vma->vm_flags & VM_GROWSDOWN) {
+ address &= PAGE_MASK;
+ if (address == vma->vm_start) {
+ struct vm_area_struct *prev = vma->vm_prev;
- /*
- * Is there a mapping abutting this one below?
- *
- * That's only ok if it's the same stack mapping
- * that has gotten split..
- */
- if (prev && prev->vm_end == address)
- return prev->vm_flags & VM_GROWSDOWN ? 0 : -ENOMEM;
+ /*
+ * Is there a ...One thing I've considered is to get rid of the CONFIG_STACK_GROWSUP
crap entirely in code, and instead just make the VM_GROWSUP #define be
0 for architectures that don't want it. The compiler should then just
automatically remove all the code that says
if (vma->vm_flags & VM_GROWSUP) {
...
and the code would look more straightforward. Hmm?
Linus
--
> But the ia64 grows-up case is tested? Yes. The attached hacky test program reports that the RSE stack stomps over the mmap'd segment w/o this patch. With it the program dies with a SIGBUS. Should be easy to adapt to You'd also need some stub declaration for expand_upwards(). But overall that would look cleaner. -Tony
Hmm. Looking at the patch a bit more..
So I react to two things:
- that "else" looks wrong. At least conceptually, both could happen -
the code should be entirely able to handle a segment that expands both
ways (which is something that ia64 could do: stack one way, register
spills etc other, all in just one happy vma). I guess we don't set it
up that way now, but the "else" just annoys my sense of aesthetics.
It's an extra four letters that don't add value - just takes it away.
- The "address = PAGE_ALIGN(address + 1);" thing is just ugly.
Wouldn't it be nicer to just move the earlier
address &= PAGE_MASK
back outside the conditionals (and keep the original conditional the
way it was - you only changed it for that bogus "else" case anyway),
and then do
if (address + PAGE_SIZE == vma->vm_end)
rather than have "PAGE_ALIGN(address + 1)" as yet another way of
aligning the address just right.
No?
Linus
--
Conceptually yes we could - I don't suppose we ever will as keeping separate vmas for stack and RSE stack defends against underflow in a program that has totally messed up. But I can save 4 bytes of This mask might be redundant ... I didn't look at the call sequence, but I've observed that "address" happens to be page Yes, that works too. Coding revised version now. -Tony --
| Greg KH | Og dreams of kernels |
| Jens Axboe | [PATCH 31/33] Fusion: sg chaining support |
| Arnd Bergmann | Re: finding your own dead "CONFIG_" variables |
| Mark Bro |
