Re: [patch 2.6.13-rc4] fix get_user_pages bug

[Ingo Molnar <mingo@elte.hu>]

* Nick Piggin <nickpiggin@yahoo.com.au> wrote:

> Ingo Molnar wrote:
> >* Nick Piggin <nickpiggin@yahoo.com.au> wrote:
> 
> >>Feedback please, anyone.
> >
> >
> >it looks good to me, but wouldnt it be simpler (in terms of patch and 
> >architecture impact) to always retry the follow_page() in 
> >get_user_pages(), in case of a minor fault? The sequence of minor faults 
> 
> I believe this can break some things. Hugh posted an example in his 
> recent post to linux-mm (ptrace setting a breakpoint in read-only 
> text). I think?

Hugh's posting said:

 "it's trying to avoid an endless loop of finding the pte not writable 
  when ptrace is modifying a page which the user is currently protected 
  against writing to (setting a breakpoint in readonly text, perhaps?)"

i'm wondering, why should that case generate an infinite fault? The 
first write access should copy the shared-library page into a private 
page and map it into the task's MM, writable. If this make-writable 
operation races with a read access then we return a minor fault and the 
page is still readonly, but retrying the write should then break up the 
COW protection and generate a writable page, and a subsequent 
follow_page() success. If the page cannot be made writable, shouldnt the 
vma flags reflect this fact by not having the VM_MAYWRITE flag, and 
hence get_user_pages() should have returned with -EFAULT earlier?

in other words, can a named MAP_PRIVATE vma with VM_MAYWRITE set ever be 
non-COW-break-able and thus have the potential to induce an infinite 
loop?

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