linux-kernel - Re: [RESEND RFC 0/2] Paravirtualized Control Register pinning

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: <03F5FE31-E769-4497-922B-C8613F0951FA@oracle.com>
Date:   Mon, 23 Dec 2019 19:28:13 +0200
From:   Liran Alon <liran.alon@...cle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     John Andersen <john.s.andersen@...el.com>, tglx@...utronix.de,
        mingo@...hat.com, bp@...en8.de, x86@...nel.org, hpa@...or.com,
        sean.j.christopherson@...el.com, vkuznets@...hat.com,
        wanpengli@...cent.com, jmattson@...gle.com, joro@...tes.org,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org
Subject: Re: [RESEND RFC 0/2] Paravirtualized Control Register pinning

> On 23 Dec 2019, at 19:09, Paolo Bonzini <pbonzini@...hat.com> wrote:
> 
> On 23/12/19 15:48, Liran Alon wrote:
>>> Should userspace expose the CR pining CPUID feature bit, it must zero CR
>>> pinned MSRs on reboot. If it does not, it runs the risk of having the
>>> guest enable pinning and subsequently cause general protection faults on
>>> next boot due to early boot code setting control registers to values
>>> which do not contain the pinned bits.
>> 
>> Why reset CR pinned MSRs by userspace instead of KVM INIT handling?
> 
> Most MSRs are not reset by INIT, are they?
> 
> Paolo
> 

MSR_KVM_SYSTEM_TIME saved in vcpu->arch.time is reset at kvmclock_reset() which is called by kvm_vcpu_reset() (KVM INIT handler).
In addition, vmx_vcpu_reset(), called from kvm_vcpu_reset(), also resets multiple MSRs such as: MSR_IA32_SPEC_CTRL (vmx->spec_ctrl) and MSR_IA32_UMWAIT_CONTROL (msr_ia32_umwait_control).

Having said that, I see indeed that most of MSRs are being set by QEMU in kvm_put_msrs() when level >= KVM_PUT_RESET_STATE.
When is triggered by qemu_system_reset() -> cpu_synchronize_all_post_reset -> cpu_synchronize_post_reset() -> kvm_cpu_synchronize_post_reset().

So given current design, OK I agree with you that CR pinned MSRs should be zeroed by userspace VMM.

It does though seems kinda weird to me that part of CPU state is initialised on KVM INIT handler and part of it in userspace VMM.
It could lead to inconsistent (i.e. diverging from spec) CPU behaviour.

-Liran