Re: [PATCH V2] Fix unsynchronized access to sev members through svm_register_enc_region

[Tom Lendacky]

On 1/27/21 3:54 PM, Sean Christopherson wrote:
> On Wed, Jan 27, 2021, Peter Gonda wrote:
> > Grab kvm->lock before pinning memory when registering an encrypted
> > region; sev_pin_memory() relies on kvm->lock being held to ensure
> > correctness when checking and updating the number of pinned pages.
...
> > +
> > +	list_add_tail(&region->list, &sev->regions_list);
> > +	mutex_unlock(&kvm->lock);
> > +
> >   	/*
> >   	 * The guest may change the memory encryption attribute from C=0 -> C=1
> >   	 * or vice versa for this memory range. Lets make sure caches are
> > @@ -1133,13 +1143,6 @@ int svm_register_enc_region(struct kvm *kvm,
> >   	 */
> >   	sev_clflush_pages(region->pages, region->npages);
>
> I don't think it actually matters, but it feels like the flush should be done
> before adding the region to the list.  That would also make this sequence
> consistent with the other flows.
>
> Tom, any thoughts?

I don't think it matters, either. This does keep the flushing outside of 
the mutex, so if you are doing parallel operations, that should help speed 
things up a bit.

Thanks,
Tom

>