linux-kernel - Re: [RFC PATCH 2/3] nSVM: introduce smv->nested.save to cache save area fields

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]

Message-ID: <82acae8f-6b27-928f-0c00-1df8fc9d26b8@redhat.com>
Date:   Tue, 28 Sep 2021 18:20:17 +0200
From:   Paolo Bonzini <pbonzini@...hat.com>
To:     Maxim Levitsky <mlevitsk@...hat.com>,
        Emanuele Giuseppe Esposito <eesposit@...hat.com>,
        kvm@...r.kernel.org
Cc:     Sean Christopherson <seanjc@...gle.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Joerg Roedel <joro@...tes.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 2/3] nSVM: introduce smv->nested.save to cache save
 area fields

On 12/09/21 12:39, Maxim Levitsky wrote:
> On Fri, 2021-09-03 at 12:20 +0200, Emanuele Giuseppe Esposito wrote:
>> This is useful in next patch, to avoid having temporary
>> copies of vmcb12 registers and passing them manually.
> 
> This is NOT what I had in mind, but I do like that idea very much,
> IMHO this is much better than what I had in mind!
> 
> The only thing that I would change is that I woudn't reuse 'struct vmcb_save_area'
> for the copy, as this both wastes space (minor issue),
> and introduces a chance of someone later using non copied
> fields from it (can cause a bug later on).
> 
> I would just define a new struct for that (but keep same names
> for readability)
> 
> Maybe something like 'struct vmcb_save_area_cached'?

I agree, I like this too.  However, it needs a comment that this new 
struct is not kept up-to-date, and is only valid until enter_svm_guest_mode.

I might even propose a

#ifdef CONFIG_DEBUG_KERNEL
	memset(&svm->nested.save, 0xaf, sizeof(svm->nested.save));
#endif

but there are no uses of CONFIG_DEBUG_KERNEL in all of Linux so it's 
probably not the way one should use that symbol.  Can anybody think of a 
similar alternative?  Or should the memset simply be unconditional?

Paolo