Re: [patch 2.6.13-rc4] fix get_user_pages bug

Linus Torvalds
2005-08-02T17:02:38Z

On Tue, 2 Aug 2005, Linus Torvalds wrote:
>
> In fact, that brings up another race altogether: a thread that does a
> fork() at the same time [...]

You don't even need that, actually. There's another race by which the
write could have gotten lost both with the new code _and_ the old code.

Since we will have dropped the page table lock when calling
handle_mm_fault() (which will just re-get the lock and then drop it
again) _and_ since we don't actually mark the page dirty if it was
writable, it's entirely possible that the VM scanner comes in and just
drops the page from the page tables.

Now, that doesn't sound so bad, but what we have then is a page that is
marked dirty in the "struct page", but hasn't been actually dirtied yet.
It could get written out and marked clean (can anybody say "preemptible
kernel"?) before we ever actually do the write to the page.

The thing is, we should always set the dirty bit either atomically with
the access (normal "CPU sets the dirty bit on write") _or_ we should set
it after the write (having kept a reference to the page).

Or does anybody see anything that protects us here?

Now, I don't think we can fix that race (which is probably pretty much
impossible to hit in practice) in the 2.6.13 timeframe.

Maybe I'll have to just accept the horrid "VM_FAULT_RACE" patch. I don't
much like it, but..

Linus