Re: [Xen-devel] [PATCH 3/3] XSA-60 security hole: cr0.cd handling

[Tim Deegan <tim@xxxxxxx>]

At 11:06 +0100 on 17 Oct (1382004394), Jan Beulich wrote:
> >>> On 16.10.13 at 20:38, "Liu, Jinsong" <jinsong.liu@xxxxxxxxx> wrote:
> > +void hvm_shadow_handle_cd(struct vcpu *v, unsigned long value)
> > +{
> > +    if ( value & X86_CR0_CD )
> > +    {
> > +        /* 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);
> 
> No matter how you order these two, there's a window during
> which new cache contents could accumulate. I think you need
> to pause all but the current vCPU around this pair of
> operations.

Yes.  Be careful, though - it's easy to cause deadlocks with that sort
of thing.  I think the safest thing to do would be to add a
domain_pause_nosync() that calls vcpu_sleep_nosync() instead of
vcpu_sleep_sync().  The IPI for on_each_cpu() will make sure the other
vcpus actually get descheduled. 

Anyway, you can add Reviewed-by: Tim Deegan <tim@xxxxxxx> to 
patches 1/3 and 2/3.

Tim.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel