Re: [PATCH] x86: do not overrun page table ranges in gup

[Linus Torvalds]

On Tue, 29 Jul 2008, Johannes Weiner wrote:
> 
> Actually, I think the prettier fix would be to just establish that
> garuantee:
>
> --- a/arch/x86/mm/gup.c
> +++ b/arch/x86/mm/gup.c
> @@ -223,7 +223,7 @@ int get_user_pages_fast(unsigned long start, int nr_pages, int write,
>  			struct page **pages)
>  {
>  	struct mm_struct *mm = current->mm;
> -	unsigned long end = start + (nr_pages << PAGE_SHIFT);
> +	unsigned long end = PAGE_ALIGN(start + (nr_pages << PAGE_SHIFT));

Umm. 'end' is guaranteed to be page-aligned if 'start' is. 

So if this makes a difference, that implies that _start_ isn't 
page-aligned, and then you when you add PAGE_SIZE to 'addr', you are going 
to miss 'end' again.

So no, the right fix would be to align 'start' first, which means that 
everything else (including 'end') will be page-aligned. Aligning just one 
or the other is very very wrong.

But yeah, this looks like a nasty bug. It's also sad that the code 
that _should_ be architecture-independent, isn't - because every 
architecture defines the _whole_ "get_user_pages_fast()", even though part 
of it is very much arch-independent (the whole alignment/access_ok part).

It also shows a bug in that whole "access_ok()" check. The fact is, that 
thing is broken too - for the same reason. If you want to get a single 
page at the end of the address space, but don't use an aligned address, 
the "access_ok()" will fail.

Nick, how do you want to fix this? I was just about to cut an -rc1, but I 
would really like to see this one not make it into it..

		Linus
--
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/