Re: [PATCH 1/1] __mlock_vma_pages_range: stack_guard_page() casereturns the wrong value

[Robert ÅwiÄcki]

On Tue, Apr 12, 2011 at 9:08 PM, Oleg Nesterov <oleg@xxxxxxxxxx> wrote:
> __mlock_vma_pages_range() simply changes addr/nr_pages when
> stack_guard_page(vma, start). But this means that __get_user_pages()
> returns a number which doesn't match the [start, end) interval and
> the caller can be confused.
>
> If we skip the first page, we should return 1 if gup fails, or add
> 1 to the number it returns.
>
> Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx>
> ---
>
> Âmm/mlock.c | Â 16 ++++++++++++----
> Â1 file changed, 12 insertions(+), 4 deletions(-)
>
> --- sigprocmask/mm/mlock.c~do_mlock_pages_stack_guard_page   Â2011-04-06 21:33:50.000000000 +0200
> +++ sigprocmask/mm/mlock.c   Â2011-04-12 20:50:30.000000000 +0200
> @@ -159,9 +159,8 @@ static long __mlock_vma_pages_range(stru
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âint *nonblocking)
> Â{
> Â Â Â Âstruct mm_struct *mm = vma->vm_mm;
> - Â Â Â unsigned long addr = start;
> Â Â Â Âint nr_pages = (end - start) / PAGE_SIZE;
> - Â Â Â int gup_flags;
> + Â Â Â int gup_flags, skip_page, ret;
>
> Â Â Â ÂVM_BUG_ON(start & ~PAGE_MASK);
>    ÂVM_BUG_ON(end  & ~PAGE_MASK);
> @@ -189,13 +188,22 @@ static long __mlock_vma_pages_range(stru
> Â Â Â Â Â Â Â Âgup_flags |= FOLL_MLOCK;
>
> Â Â Â Â/* We don't try to access the guard page of a stack vma */
> + Â Â Â skip_page = 0;
> Â Â Â Âif (stack_guard_page(vma, start)) {
> - Â Â Â Â Â Â Â addr += PAGE_SIZE;
> + Â Â Â Â Â Â Â skip_page = 1;
> + Â Â Â Â Â Â Â start += PAGE_SIZE;
> Â Â Â Â Â Â Â Ânr_pages--;
> Â Â Â Â}
>
> - Â Â Â return __get_user_pages(current, mm, addr, nr_pages, gup_flags,
> + Â Â Â ret = __get_user_pages(current, mm, start, nr_pages, gup_flags,
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â ÂNULL, NULL, nonblocking);
> +
> + Â Â Â if (ret >= 0)
> + Â Â Â Â Â Â Â ret += skip_page;
> + Â Â Â else if (skip_page)
> + Â Â Â Â Â Â Â ret = 1;
> +
> + Â Â Â return ret;
> Â}
>
> Â/*

Compiling with Linus' new patch now, lemme know if you agree on which
one might be the more correct one :). Otherwise I'll stick to the
first choice, and let you know tomorrow if it worked some more
extensive testing.

-- 
Robert ÅwiÄcki
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/