Re: [PATCH] ppc64: Fix possible race with set_pte on a present PTE

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

> Ahh.. That's a bug, methinks.
> 
> The reason it's a bug is that if you do this, you can lose the dirty bit 
> being written on some other CPU asynchronously.

Hrm... right indeed.

> In other words, I think it's pretty much always a bug to do a "set_pte()"
> with an existing pte in place, exactly because you lose information. You
> are trying to cover up the bug in ppc64-specific code, but I think that
> what you found is actually a (really really) unlikely race condition that
> can have serious consequences.
> 
> Or am I missing something else?

Well, the original scenario triggering that from userland is, imho, so
broken, that we may just not care losing that dirty bit ... Oh well :)
Anyway, apply my patch. If pte is not present, this will have no effect,
if it is, it makes sure we never leave a stale HPTE in the hash, which
is fatal in far worse ways.

> [ grep grep grep ]
> 
> Looks like "break_cow()" and "do_wp_page()" are safe, but only because
> they always sets the dirty bit, and any other bits end up being pretty 
> much "don't care if we miss an accessed bit update" or something.
> 
> Hmm. Maybe I'm wrong. If this really is buggy, it's been buggy this way 
> basically forever. That code is _not_ new, it's some of the oldes code in 
> the whole VM since the original three-level code rewrite. I think. Of 
> course, back then SMP wasn't an issue, and this seems to have survived all 
> the SMP fixes.
> 
> Who else has been working on the page tables that could verify this for 
> me? Ingo? Ben LaHaise? I forget who even worked on this, because it's so 
> long ago we went through all the atomicity issues with the page table 
> updates on SMP. There may be some reason that I'm overlooking that 
> explains why I'm full of sh*t.

Ben.