Re: [patch 10/12] [PATCH] kvm-s390: storage key interface

[Heiko Carstens <heiko.carstens@xxxxxxxxxx>]

On Sat, Dec 10, 2011 at 01:35:39PM +0100, Carsten Otte wrote:
> This patch introduces an interface to access the guest visible
> storage keys. It supports three operations that model the behavior
> that SSKE/ISKE/RRBE instructions would have if they were issued by
> the guest. These instructions are all documented in the z architecture
> principles of operation book.
> 
> Signed-off-by: Carsten Otte <cotte@xxxxxxxxxx>

[...]

> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -112,13 +112,115 @@ void kvm_arch_exit(void)
>  {
>  }
> 
> +static long kvm_s390_keyop(struct kvm_s390_keyop *kop)
> +{
> +	unsigned long addr = kop->user_addr;
> +	pte_t *ptep;
> +	pgste_t pgste;
> +	int r;
> +	unsigned long skey;
> +	unsigned long bits;
> +
> +	/* make sure this process is a hypervisor */
> +	r = -EINVAL;
> +	if (!mm_has_pgste(current->mm))
> +		goto out;
> +
> +	r = -EFAULT;
> +	if (addr >= PGDIR_SIZE)
> +		goto out;
> +
> +	spin_lock(&current->mm->page_table_lock);
> +	ptep = ptep_for_addr(addr);

Locking is broken; following order is possible:

kvm_s390_keyop()    - spin_lock(&current->mm->page_table_lock)
-> ptep_for_addr()  - down_read(&current->mm->mmap_sem)
                      ---> Bug 1, we might schedule here
-> __pmdp_for_addr()
-> __pte_alloc()    - spin_lock(&mm->page_table_lock)
                      ---> Bug 2, deadlock

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html