Re: [patch 2.6.13-rc4] fix get_user_pages bug

[Nick Piggin <nickpiggin@yahoo.com.au>]

On Mon, 2005-08-01 at 20:45 -0700, Linus Torvalds wrote:
> 
> On Tue, 2 Aug 2005, Nick Piggin wrote:
> > 
> > Surely this introduces integrity problems when `force` is not set?
> 
> "force" changes how we test the vma->vm_flags, that was always the 
> meaning from a security standpoint (and that hasn't changed).
> 

Of course, this test catches the problem I had in mind.

> The old code had this "lookup_write = write && !force;" thing because
> there it used "force" to _clear_ the write bit test, and that was what
> caused the race in the first place - next time around we would accept a
> non-writable page, even if it hadn't actually gotten COW'ed.
> 
> So no, the patch doesn't introduce integrity problems by ignoring "force".  
> Quite the reverse - it _removes_ the integrity problems by ignoring it
> there. That's kind of the whole point.
> 

OK, I'm convinced. One last thing - your fix might have a non
trivial overhead in terms of spin locks and simply entering the
high level page fault handler when dealing with clean, writeable
ptes for write.

Any chance you can change the __follow_page test to account for
writeable clean ptes? Something like

	if (write && !pte_dirty(pte) && !pte_write(pte))
		goto out;

And then you would re-add the set_page_dirty logic further on.

Not that I know what Robin's customer is doing exactly, but it
seems like something you can optimise easily enough.

Nick

-- 
SUSE Labs, Novell Inc.



Send instant messages to your online friends http://au.messenger.yahoo.com 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/