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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <alpine.LFD.1.10.0807281721200.3486@nehalem.linux-foundation.org>
Date:	Mon, 28 Jul 2008 17:33:46 -0700 (PDT)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Johannes Weiner <hannes@...urebad.de>
cc:	Alexey Dobriyan <adobriyan@...il.com>, akpm@...uxfoundation.org,
	torvalds@...uxfoundation.org, npiggin@...e.de,
	linux-kernel@...r.kernel.org
Subject: Re: [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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/