On 06.01.2021 20:03, Ben Gardon wrote:
On Wed, Jan 6, 2021 at 10:59 AM Ben Gardon <bgardon@xxxxxxxxxx> wrote:Many TDP MMU functions which need to perform some action on all TDP MMU roots hold a reference on that root so that they can safely drop the MMU lock in order to yield to other threads. However, when releasing the reference on the root, there is a bug: the root will not be freed even if its reference count (root_count) is reduced to 0. To simplify acquiring and releasing references on TDP MMU root pages, and to ensure that these roots are properly freed, move the get/put operations into the TDP MMU root iterator macro. Not all functions which use the macro currently get and put a reference to the root, but adding this behavior is harmless. Moving the get/put operations into the iterator macro also helps simplify control flow when a root does need to be freed. Note that using the list_for_each_entry_unsafe macro would not have been appropriate in this situation because it could keep a reference to the next root across an MMU lock release + reacquire. Reported-by: Maciej S. Szmigiero <maciej.szmigiero@xxxxxxxxxx> Suggested-by: Paolo Bonzini <pbonzini@xxxxxxxxxx> Fixes: faaf05b00aec ("kvm: x86/mmu: Support zapping SPTEs in the TDP MMU") Fixes: 063afacd8730 ("kvm: x86/mmu: Support invalidate range MMU notifier for TDP MMU") Fixes: a6a0b05da9f3 ("kvm: x86/mmu: Support dirty logging for the TDP MMU") Fixes: 14881998566d ("kvm: x86/mmu: Support disabling dirty logging for the tdp MMU") Signed-off-by: Ben Gardon <bgardon@xxxxxxxxxx> ---
(..)
I tested v2 with Maciej's test (https://urldefense.com/v3/__https://gist.github.com/maciejsszmigiero/890218151c242d99f63ea0825334c6c0__;!!GqivPVa7Brio!NUh8Xbu1YkhSf49HvbyhI-svvPmJXWj9KECqaEd7ZJMKPdz-HdND1sKduH2VpwasEN8Gpg$ , near the bottom of the page) on an Intel Skylake Machine and can confirm that v1 failed the test but v2 passes. The problem with v1 was that roots were being removed from the list before list_next_entry was called, resulting in a bad value.
I've tested the fix now and can confirm, too, that I can no longer observe any crash. Thanks, Maciej
