Re: [patch] mm: fix anon_vma races

Linus Torvalds
2008-10-18T02:53:49Z

On Sat, 18 Oct 2008, Nick Piggin wrote:
> @@ -171,6 +181,10 @@ static struct anon_vma *page_lock_anon_v
>
> anon_vma = (struct anon_vma *) (anon_mapping - PAGE_MAPPING_ANON);
> spin_lock(&anon_vma->lock);
> +
> + if (anon_mapping != (unsigned long)page->mapping)
> + goto out;
> +
> return anon_vma;
> out:
> rcu_read_unlock();

I see why you'd like to try to do this, but look a bit closer, and you'll
realize that this is *really* wrong.

So there's the brown-paper-bag-reason why it's wrong: you need to unlock
in this case, but there's a subtler reason why I doubt the whole approach
works: I don't think we actually hold the anon_vma lock when we set
page->mapping.

So I don't think you really fixed the race that you want to fix, and I
don't think that does what you wanted to do.

But I might have missed something.

I'm off to play poker. It's Friday night, there's only so many memory
ordering and locking issues I can take in one day. I'm hoping that by the
time I look at this again, you and Hugh will have sorted it out.

Linus