linux-kernel - Re: [PATCH 2/2] KVM: x86: Use memcmp in kvm_cpuid_check

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]

Date:   Mon, 24 Jan 2022 19:08:38 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Vitaly Kuznetsov <vkuznets@...hat.com>, kvm@...r.kernel.org,
        Wanpeng Li <wanpengli@...cent.com>,
        Jim Mattson <jmattson@...gle.com>,
        Igor Mammedov <imammedo@...hat.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] KVM: x86: Use memcmp in kvm_cpuid_check_equal()

On Mon, Jan 24, 2022, Paolo Bonzini wrote:
> On 1/24/22 17:52, Sean Christopherson wrote:
> > On Mon, Jan 24, 2022, Vitaly Kuznetsov wrote:
> > > Paolo Bonzini <pbonzini@...hat.com> writes:
> > > > > +	if (memcmp(e2, vcpu->arch.cpuid_entries, nent * sizeof(*e2)))
> > > > > +		return -EINVAL;
> > > > 
> > > > Hmm, not sure about that due to the padding in struct kvm_cpuid_entry2.
> > > >    It might break userspace that isn't too careful about zeroing it.
> > 
> > Given that we already are fully committed to potentially breaking userspace by
> > disallowing KVM_SET_CPUID{2} after KVM_RUN, we might as well get greedy.
> 
> Hmm, I thought this series was because we were _not_ fully committed. :)

We're fully committed in the sense that we know disallowing KVM_SET_CPUID2 after
KVM_RUN broke at least one use case, and instead of reverting we are doubling down
and adding more KVM code/complexity to grandfather in that one use case.  There's
no guarantee that there aren't other use cases that will break, but haven't been
reported simply because their users haven't yet moved to a 5.16 kernel.