On Thu, 25 Jan 2007, Adam Litke wrote:
> When expanding the stack, we don't currently check if the VMA will cross into
> an area of the address space that is reserved for hugetlb pages. Subsequent
> faults on the expanded portion of such a VMA will confuse the low-level MMU
> code, resulting in an OOPS. Check for this.
Good point, and your patch looks good; but a couple of reservations.
Trivial: I'd have preferred your error return to be -ENOMEM like all
the rest there, I can't see a good reason for this one to say -EFAULT.
Though apparently nothing cares just so long as it's non-zero.
Less trivial (and I wonder whether you've come to this from an ia64
or a powerpc direction): I notice that ia64 has more stringent REGION
checks in its ia64_do_page_fault, before calling expand_stack or
expand_upwards. So on that path, the usual path, I think your
new check in acct_stack_growth is unnecessary on ia64; whereas
coming from the get_user_pages find_extend_vma direction,
your check is inadequate?
I'd dearly like to believe that get_user_pages shouldn't be needing
find_extend_vma (it ends up using the wrong task's rlimits); but last
time I researched that, it looked very deliberate, that ptrace be
allowed to expand the stack - I think there's something out there
which would break if that weren't allowed, but I don't know what.
Hugh
>
> Signed-off-by: Adam Litke <[EMAIL PROTECTED]>
> ---
>
> mm/mmap.c | 7 +++++++
> 1 files changed, 7 insertions(+), 0 deletions(-)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 9717337..2c6b163 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1477,6 +1477,7 @@ static int acct_stack_growth(struct vm_area_struct *
> vma, unsigned long size, un
> {
> struct mm_struct *mm = vma->vm_mm;
> struct rlimit *rlim = current->signal->rlim;
> + unsigned long new_start;
>
> /* address space limit tests */
> if (!may_expand_vm(mm, grow))
> @@ -1496,6 +1497,12 @@ static int acct_stack_growth(struct vm_area_struct *
> vma, unsigned long size, un
> return -ENOMEM;
> }
>
> + /* Check to make the stack will not grow into a hugetlb-only region. */
> + new_start = (vma->vm_flags & VM_GROWSUP) ? vma->vm_start :
> + vma->vm_end - size;
> + if (is_hugepage_only_range(vma->vm_mm, new_start, size))
> + return -EFAULT;
> +
> /*
> * Overcommit.. This must be the final test, as it will
> * update security statistics.
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to [EMAIL PROTECTED] For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: <a href=mailto:"[EMAIL PROTECTED]"> [EMAIL PROTECTED] </a>
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
