linux-kernel - Re: [PART1 RFC v4 09/11] svm: Do not expose x2APIC when enable AVIC

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: <570D720C.8020603@redhat.com>
Date:	Wed, 13 Apr 2016 00:09:16 +0200
From:	Paolo Bonzini <pbonzini@...hat.com>
To:	Radim Krčmář <rkrcmar@...hat.com>,
	Suravee Suthikulpanit <Suravee.Suthikulpanit@....com>
Cc:	joro@...tes.org, bp@...en8.de, gleb@...nel.org,
	alex.williamson@...hat.com, kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org, wei@...hat.com,
	sherry.hurwitz@....com
Subject: Re: [PART1 RFC v4 09/11] svm: Do not expose x2APIC when enable AVIC



On 11/04/2016 22:54, Radim Krčmář wrote:
>> >  
>> >  static void svm_set_supported_cpuid(u32 func, struct kvm_cpuid_entry2 *entry)
>> >  {
>> >  	switch (func) {
>> > +	case 0x00000001:
> ("case 1:" or "case 0x1:" would be easier to read.)
> 
>> > +		if (avic)
>> > +			entry->ecx &= ~bit(X86_FEATURE_X2APIC);
>> > +		break;
> 
> ---
> A rant for the unlikely case I get back to fix the broader situation:
> Only one of these two additions is needed.  If we do the second one,
> then userspace should not set X2APIC, therefore the first one is
> useless.
> 
> Omitting the second one allows userspace to clear apicv_active and set
> X86_FEATURE_X2APIC, but it needs a non-intuitive order of ioctls, so I
> think we should have the second one.
> 
> The problem is that KVM doesn't seems to check whether userspace sets
> cpuid that is a subset of supported ones, so omitting the first one
> needlessly expands the space for potential failures.

Yes, we need both.

Paolo