On Mon, 06 May 2013 13:50:18 +0800 Xiao Guangrong <xiaoguangrong@xxxxxxxxxxxxxxxxxx> wrote: > The commit 751efd8610d3 (mmu_notifier_unregister NULL Pointer deref > and multiple ->release()) breaks the fix: > 3ad3d901bbcfb15a5e4690e55350db0899095a68 > (mm: mmu_notifier: fix freed page still mapped in secondary MMU) > > Since hlist_for_each_entry_rcu() is changed now, we can not revert that patch > directly, so this patch reverts the commit and simply fix the bug spotted > by that patch > > This bug spotted by commit 751efd8610d3 is: > ====== > There is a race condition between mmu_notifier_unregister() and > __mmu_notifier_release(). > > Assume two tasks, one calling mmu_notifier_unregister() as a result of a > filp_close() ->flush() callout (task A), and the other calling > mmu_notifier_release() from an mmput() (task B). > > A B > t1 srcu_read_lock() > t2 if (!hlist_unhashed()) > t3 srcu_read_unlock() > t4 srcu_read_lock() > t5 hlist_del_init_rcu() > t6 synchronize_srcu() > t7 srcu_read_unlock() > t8 hlist_del_rcu() <--- NULL pointer deref. > ====== > > This can be fixed by using hlist_del_init_rcu instead of hlist_del_rcu. > > The another issue spotted in the commit is > "multiple ->release() callouts", we needn't care it too much because > it is really rare (e.g, can not happen on kvm since mmu-notify is unregistered > after exit_mmap()) and the later call of multiple ->release should be > fast since all the pages have already been released by the first call. > Anyway, this issue should be fixed in a separate patch. The 751efd8610d3 changelog failed to describe how these duplicated ->release calls can occur. Races with concurrent notifier registrations, I assume? > -stable suggestions: > Any version has commit 751efd8610d3 need to be backported. I find the oldest > version has this commit is 3.0-stable. > > ... > > Andrew, this patch has been tested by Robin and the test shows that the bug > of "NULL Pointer deref" bas been fixed. However, we have the argument that > whether the fix of "multiple ->release" should be merged into this patch. > (This patch just do fix the bug of "NULL Pointer deref") > > Your thought? Insufficient information :( -- To unsubscribe, send a message with 'unsubscribe linux-mm' in the body to majordomo@xxxxxxxxx. For more info on Linux MM, see: http://www.linux-mm.org/ . Don't email: <a href=mailto:"dont@xxxxxxxxx";> email@xxxxxxxxx </a>