[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH] XSA-60 security hole: cr0.cd handling
>>> On 15.10.13 at 18:47, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote: > Jan Beulich wrote: >>>>> On 14.10.13 at 11:28, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote: >>> Jan Beulich wrote: >>>>>>> On 11.10.13 at 20:25, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> >>>>>>> wrote: >>>>> +void hvm_handle_cd_traditional(struct vcpu *v, unsigned long >>>>> value) +{ + if ( (value & X86_CR0_CD) && !(value & X86_CR0_NW) >>>>> ) + { + /* Entering no fill cache mode. */ >>>>> + spin_lock(&v->domain->arch.hvm_domain.uc_lock); >>>>> + v->arch.hvm_vcpu.cache_mode = NO_FILL_CACHE_MODE; + >>>>> + if ( !v->domain->arch.hvm_domain.is_in_uc_mode ) + >>>>> { + /* Flush physical caches. */ >>>>> + on_each_cpu(local_flush_cache, NULL, 1); >>>>> + hvm_set_uc_mode(v, 1); >>>> >>>> So you continue to use the problematic function, and you also >>>> don't remove the problematic code from it's VMX backend - what >>>> am I missing here? If there was no room for getting here with >>>> !paging_mode_hap(), the problematic code should be removed >>>> from vmx_set_uc_mode(). And if we still can get here with >>>> !paging_mode_hap(), then what's the purpose of the patch?' >>>> >>> >>> The new solution w/ PAT depends on whether Intel processor support >>> 'load IA32_PAT' VM-entry control bit (which in very old Intel >>> processors it indeed didn't support it). >>> >>> Though I'm pretty sure Intel processors w/ EPT capability also >>> support 'load IA32_PAT' VM-entry control bit, Intel SDM didn't >>> explicitly guarantee it. Hence we still keep old function, though >>> I'm sure it will never be called, just for safety or logic >>> integration. >> >> If any such processor exists, the security issue would still be there >> with the patch as is. Hence either use of EPT needs to be >> suppressed in that case, or some other solution allowing the broken >> code to be deleted (or replaced) needs to be found. >> > > Well, I have to go back to Intel library:) a long search for Intel VT > history: both 'load IA32_PAT' and 'EPT' features does not exist at Intel SDM > Order Number: 253669-026US (Feb 2008), but co-exist as early as Intel SDM > Order > Number: 253669-027US (July 2008). > > Per my understanding 'load IA32_PAT' co-exist/co-work with EPT: > 1. shadow don't need 'load IA32_PAT' feature > 2. EPT, from its earliest version, has 'iPAT' bit to control guest PAT and > host EPT memory type combining -- without 'load IA32_PAT' how can guest PAT > memory type take effect? > > So we have 2 choices for 'load IA32_PAT': > 1. aggressive choice: remove vmx_set_uc_mode logic > if ( shadow ) > drop shadows and new ones would be created on demand; > else /* Intel EPT */ > IA32_PAT solution > 2. conservative choice: as what the patch did, use if (!cpu_has_vmx_pat) to > keep logic integration -- it didn't make thing worse. As said before - the code in question needs to go away or be modified in a way that's safe. If there are extra restrictions that need to be enforced (like cpu_has_vmx_pat being a prereq for EPT+non-snooped-IOMMU), then so be it (i.e. your patch would need to be extended to do that). Jan _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx http://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |