linux-kernel - Re: [PATCH 00/21] mm: Preemptibility -v6

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sun, 23 Jan 2011 12:03:50 +0100
From:	Peter Zijlstra <a.p.zijlstra@...llo.nl>
To:	paulmck@...ux.vnet.ibm.com
Cc:	Hugh Dickins <hughd@...gle.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	David Miller <davem@...emloft.net>,
	Nick Piggin <npiggin@...nel.dk>,
	Martin Schwidefsky <schwidefsky@...ibm.com>,
	linux-kernel@...r.kernel.org, linux-arch@...r.kernel.org,
	linux-mm@...ck.org, Andrea Arcangeli <aarcange@...hat.com>,
	Oleg Nesterov <oleg@...hat.com>
Subject: Re: [PATCH 00/21] mm: Preemptibility -v6

On Sat, 2011-01-22 at 13:06 -0800, Paul E. McKenney wrote:

> OK, so the anon_vma slab cache is SLAB_DESTROY_BY_RCU.  Presumably
> all callers of page_lock_anon_vma() check the identity of the page
> that got locked, since it might be recycled at any time.  But when
> I look at 2.6.37, I only see checks for NULL.  So I am assuming
> that this code is supposed to prevent such recycling.
> 
> I am not sure that I am seeing a consistent snapshot of all of the
> relevant code, in particular, I am guessing that the ->lock and ->mutex
> are the result of changes rather than there really being both a spinlock
> and a mutex in anon_vma. 

Correct, my earlier spinlock -> mutex conversion left is being called
->lock, but Hugh (rightly) pointed out that I should rename it too, so
in the new (as of yet unposted version its called ->mutex).

>  Mainline currently has a lock, FWIW.  But from
> what I do see, I am concerned about the following sequence of events:
> 
> o	CPU 0 starts executing page_lock_anon_vma() as shown at
> 	https://lkml.org/lkml/2010/11/26/213, fetches the pointer
> 	to anon_vma->root->lock, but does not yet invoke
> 	mutex_trylock().
> 
> o	CPU 1 executes __put_anon_vma() above on the same VMA
> 	that CPU 0 is attempting to use.  It sees that the
> 	anon_vma->root->mutex (presumably AKA ->lock) is not held,
> 	so it calls anon_vma_free().
> 
> o	CPU 2 reallocates the anon_vma freed by CPU 1, so that it
> 	now has a non-zero reference count.
> 
> o	CPU 0 continues execution, incorrectly acquiring a reference
> 	to the now-recycled anon_vma.
> 
> Or am I misunderstanding what this code is trying to do?

No that is quite right and possible, its one of the many subtle issues
surrounding the existing page_lock_anon_vma(), we can indeed return a
locked anon_vma that is not in fact related to the page we asked it for,
all calling code SHOULD and afaict does deal with that, mostly by
calling things like vma_address(vma, page) for all vma's obtained from
the anon_vma, to verify the page is indeed (or not) part of the vma.

The race we guard against with all the fancy stuff is the page itself
getting unmapped and us returning an anon_vma for an unmapped page.

And of course, returning a locked but free'd anon_vma, that too isn't
allowed ;-)


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/