linux-kernel - Re: [PATCH] KVM: x86/mmu: Set "shadow_root_alloced" accordingly when TDP is disabled

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] [day] [month] [year] [list]

Message-ID: <083776a1-cec7-7a67-265b-a69e2a784cca@redhat.com>
Date:   Mon, 18 Oct 2021 19:51:28 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: x86/mmu: Set "shadow_root_alloced" accordingly when
 TDP is disabled

On 18/10/21 19:47, Sean Christopherson wrote:
> Explicitly check kvm_shadow_root_alloced() when short-circuiting shadow
> paging metadata allocations and skip setting "shadow_root_alloced" if and
> only if its already true, i.e. set it when short-circuiting because TDP is
> disabled.  This fixes a benign bug where KVM would always take
> slots_arch_lock when allocating a shadow root due to "shadow_root_alloced"
> never being set.
> 
> Opportunistically add comments to call out that not freeing successful
> allocations on failure is intentional, and that freeing on failure isn't
> straightforward so as to discourage incorrect cleanups in the future.
> 
> Fixes: 73f122c4f06f ("KVM: cleanup allocation of rmaps and page tracking data")
> Signed-off-by: Sean Christopherson<seanjc@...gle.com>
> ---
> 
> Essentially code review for "KVM: cleanup allocation of rmaps and page
> tracking data", which AFAICT didn't get posted (because it came in via a
> a merge?).

It didn't get posted because it is not merged yet - it's basically David 
Steven's v3 merged into kvm/queue for him to take a look at all the 
kvm/master and kvm/next juggling.  Thanks for looking at it already, 
I've squashed the fix in and will post it shortly.

Paolo