lkml.org 
[lkml]   [2008]   [Jul]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] x86: do not overrun page table ranges in gup


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


\
 
 \ /
  Last update: 2008-07-29 02:39    [W:0.094 / U:0.516 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site