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: <fecafe83-5f45-a0ab-c208-9ea4720fcb6a@amd.com>
Date:   Wed, 27 Jan 2021 16:51:56 -0600
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Sean Christopherson <seanjc@...gle.com>,
        Peter Gonda <pgonda@...gle.com>
Cc:     kvm@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Joerg Roedel <joro@...tes.org>,
        Brijesh Singh <brijesh.singh@....com>, x86@...nel.org,
        stable@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V2] Fix unsynchronized access to sev members through
 svm_register_enc_region

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

> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ