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

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Mon, 2014-07-28 at 14:11 +1000, Alexey Kardashevskiy wrote:
> ===
> 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?

Well, not really, we'd need to see the corresponding RCU'isms when
walking the list including the rcu read lock ... which could be
problematic if we are in real mode....

Cheers,
Ben.


> 
> 
> > 
> > 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);
> >>  
> > 
> > 
> 
>