Subject: Re: [PATCH v3 02/18] KVM: PPC: Use RCU when adding to arch.spapr_tce_tables

From: Alexey Kardashevskiy <[email-masked]>
Date: Mon, 28 Jul 2014 14:11:01 +1000

On 07/28/2014 10:40 AM, Benjamin Herrenschmidt wrote:
> On Thu, 2014-07-24 at 18:47 +1000, Alexey Kardashevskiy wrote:
>> Signed-off-by: Alexey Kardashevskiy <[email-masked]>
>> Reviewed-by: Gavin Shan <[email-masked]>
> 
> You need a much better changeset here, explaining what you are
> protecting against and why just changing that one list_add is
> sufficient.

===
At the moment the spapr_tce_tables list is not protected against races
which may happen if the userspace issues the KVM_CREATE_SPAPR_TCE ioctl
to KVM from different threads.

This makes use of _rcu helpers for list_add()/list_del().
===

and it is missing this bit as well (was under impression that all tables
get freed at once at KVM exit but this might not be the case for malicious
guest):

--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -211,7 +211,7 @@ static void release_spapr_tce_table(struct
kvmppc_spapr_tce_table *stt)
        long npages = kvmppc_stt_npages(stt->size);

        mutex_lock(&kvm->lock);
-       list_del(&stt->list);
+       list_del_rcu(&stt->list);

Would this be sufficient?

> 
> Cheers,
> Ben.
> 
>> ---
>>  arch/powerpc/kvm/book3s_64_vio.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
>> index 54cf9bc..516f2ee 100644
>> --- a/arch/powerpc/kvm/book3s_64_vio.c
>> +++ b/arch/powerpc/kvm/book3s_64_vio.c
>> @@ -131,7 +131,7 @@ long kvm_vm_ioctl_create_spapr_tce(struct kvm *kvm,
>>  	kvm_get_kvm(kvm);
>>  
>>  	mutex_lock(&kvm->lock);
>> -	list_add(&stt->list, &kvm->arch.spapr_tce_tables);
>> +	list_add_rcu(&stt->list, &kvm->arch.spapr_tce_tables);
>>  
>>  	mutex_unlock(&kvm->lock);
>>  
> 
> 

-- 
Alexey