linux-kernel - Re: [PATCH] mm: fix possible cause of a page

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.LSU.2.00.1104060837590.4909@sister.anvils>
Date:	Wed, 6 Apr 2011 08:43:43 -0700 (PDT)
From:	Hugh Dickins <hughd@...gle.com>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
cc:	Hugh Dickins <hughd@...gle.com>,
	Robert Święcki <robert@...ecki.net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Miklos Szeredi <miklos@...redi.hu>,
	Michel Lespinasse <walken@...gle.com>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Rik van Riel <riel@...hat.com>
Subject: Re: [PATCH] mm: fix possible cause of a page_mapped BUG

On Wed, 6 Apr 2011, Linus Torvalds wrote:
> On Wed, Apr 6, 2011 at 7:47 AM, Hugh Dickins <hughd@...gle.com> wrote:
> >>
> >> I dunno. But that odd negative pg_off thing makes me think there is
> >> some overflow issue (ie HEAP_INDEX being pg_off + size ends up
> >> fluctuating between really big and really small). So I'd suspect THAT
> >> as the main reason.
> >
> > Yes, one of the vmas is such that the end offset (pgoff of next page
> > after) would be 0, and for the other it would be 16.  There's sure to
> > be places, inside the prio_tree code and outside it, where we rely
> > upon pgoff not wrapping around - wrap should be prevented by original
> > validation of arguments.
> 
> Well, we _do_ validate them in do_mmap_pgoff(), which is the main
> routine for all the mmap() system calls, and the main way to get a new
> mapping.
> 
> There are other ways, like do_brk(), but afaik that always sets
> vm_pgoff to the virtual address (shifted), so again the new mapping
> should be fine.
> 
> So when a new mapping is created, it should all be ok.
> 
> But I think mremap() may end up expanding it without doing the same
> overflow check.
> 
> Do you see any other way to get this situation? Does the vma dump give
> you any hint about where it came from?
> 
> Robert - here's a (UNTESTED!) patch to make mremap() be a bit more
> careful about vm_pgoff when growing a mapping. Does it make any
> difference?

I'd come to the same conclusion: the original page_mapped BUG has itself
suggested that mremap() is getting used.

I was about to send you my own UNTESTED patch: let me append it anyway,
I think it is more correct than yours (it's the offset of vm_end we need
to worry about, and there's the funny old_len,new_len stuff).  See what
you think - sorry, I'm going out now.

Hugh

--- 2.6.38/mm/mremap.c	2011-03-14 18:20:32.000000000 -0700
+++ linux/mm/mremap.c	2011-04-06 08:31:46.000000000 -0700
@@ -282,6 +282,12 @@ static struct vm_area_struct *vma_to_res
 			goto Efault;
 	}
 
+	if (vma->vm_file && new_len > old_len) {
+		pgoff_t endoff = linear_page_index(vma, vma->vm_end);
+		if (endoff + ((new_len - old_len) >> PAGE_SHIFT) < endoff)
+			goto Eoverflow;
+	}
+
 	if (vma->vm_flags & VM_LOCKED) {
 		unsigned long locked, lock_limit;
 		locked = mm->locked_vm << PAGE_SHIFT;
@@ -311,6 +317,8 @@ Enomem:
 	return ERR_PTR(-ENOMEM);
 Eagain:
 	return ERR_PTR(-EAGAIN);
+Eoverflow:
+	return ERR_PTR(-EOVERFLOW);
 }
 
 static unsigned long mremap_to(unsigned long addr,