[GIT PULL 0/2] KVM: s390: Fixes for 3.17

[GIT PULL 0/2] KVM: s390: Fixes for 3.17

[Christian Borntraeger]

Paolo,

the following changes since commit 7d1311b93e58ed55f3a31cc8f94c4b8fe988a2b9:

  Linux 3.17-rc1 (2014-08-16 10:40:26 -0600)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git  tags/kvm-s390-20140825

for you to fetch changes up to ab3f285f227fec62868037e9b1b1fd18294a83b8:

  KVM: s390/mm: try a cow on read only pages for key ops (2014-08-25 14:35:28 +0200)

----------------------------------------------------------------
Here are two fixes for s390 KVM code that prevent:
1. a malicious user to trigger a kernel BUG
2. a malicious user to change the storage key of read-only pages

----------------------------------------------------------------
Christian Borntraeger (2):
      KVM: s390: Fix user triggerable bug in dead code
      KVM: s390/mm: try a cow on read only pages for key ops

 arch/s390/kvm/kvm-s390.c | 13 -------------
 arch/s390/mm/pgtable.c   | 10 ++++++++++
 2 files changed, 10 insertions(+), 13 deletions(-)

[GIT PULL 1/2] KVM: s390: Fix user triggerable bug in dead code

[Christian Borntraeger]

In the early days, we had some special handling for the
KVM_EXIT_S390_SIEIC exit, but this was gone in 2009 with commit
d7b0b5eb3000 (KVM: s390: Make psw available on all exits, not
just a subset).

Now this switch statement is just a sanity check for userspace
not messing with the kvm_run structure. Unfortunately, this
allows userspace to trigger a kernel BUG. Let's just remove
this switch statement.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Cc: stable@vger.kernel.org
---
 arch/s390/kvm/kvm-s390.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index ce81eb2..81b0e11 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -1317,19 +1317,6 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *kvm_run)
 		return -EINVAL;
 	}
 
-	switch (kvm_run->exit_reason) {
-	case KVM_EXIT_S390_SIEIC:
-	case KVM_EXIT_UNKNOWN:
-	case KVM_EXIT_INTR:
-	case KVM_EXIT_S390_RESET:
-	case KVM_EXIT_S390_UCONTROL:
-	case KVM_EXIT_S390_TSCH:
-	case KVM_EXIT_DEBUG:
-		break;
-	default:
-		BUG();
-	}
-
 	vcpu->arch.sie_block->gpsw.mask = kvm_run->psw_mask;
 	vcpu->arch.sie_block->gpsw.addr = kvm_run->psw_addr;
 	if (kvm_run->kvm_dirty_regs & KVM_SYNC_PREFIX) {
-- 
1.8.4.2

[GIT PULL 2/2] KVM: s390/mm: try a cow on read only pages for key ops

[Christian Borntraeger]

The PFMF instruction handler  blindly wrote the storage key even if
the page was mapped R/O in the host. Lets try a COW before continuing
and bail out in case of errors.

Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Reviewed-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
Cc: stable@vger.kernel.org
---
 arch/s390/mm/pgtable.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
index 19daa53..5404a62 100644
--- a/arch/s390/mm/pgtable.c
+++ b/arch/s390/mm/pgtable.c
@@ -986,11 +986,21 @@ int set_guest_storage_key(struct mm_struct *mm, unsigned long addr,
 	pte_t *ptep;
 
 	down_read(&mm->mmap_sem);
+retry:
 	ptep = get_locked_pte(current->mm, addr, &ptl);
 	if (unlikely(!ptep)) {
 		up_read(&mm->mmap_sem);
 		return -EFAULT;
 	}
+	if (!(pte_val(*ptep) & _PAGE_INVALID) &&
+	     (pte_val(*ptep) & _PAGE_PROTECT)) {
+			pte_unmap_unlock(*ptep, ptl);
+			if (fixup_user_fault(current, mm, addr, FAULT_FLAG_WRITE)) {
+				up_read(&mm->mmap_sem);
+				return -EFAULT;
+			}
+			goto retry;
+		}
 
 	new = old = pgste_get_lock(ptep);
 	pgste_val(new) &= ~(PGSTE_GR_BIT | PGSTE_GC_BIT |
-- 
1.8.4.2

Re: [GIT PULL 0/2] KVM: s390: Fixes for 3.17

[Paolo Bonzini]

Il 25/08/2014 15:10, Christian Borntraeger ha scritto:
> Paolo,
> 
> the following changes since commit 7d1311b93e58ed55f3a31cc8f94c4b8fe988a2b9:
> 
>   Linux 3.17-rc1 (2014-08-16 10:40:26 -0600)
> 
> are available in the git repository at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git  tags/kvm-s390-20140825
> 
> for you to fetch changes up to ab3f285f227fec62868037e9b1b1fd18294a83b8:
> 
>   KVM: s390/mm: try a cow on read only pages for key ops (2014-08-25 14:35:28 +0200)
> 
> ----------------------------------------------------------------
> Here are two fixes for s390 KVM code that prevent:
> 1. a malicious user to trigger a kernel BUG
> 2. a malicious user to change the storage key of read-only pages
> 
> ----------------------------------------------------------------
> Christian Borntraeger (2):
>       KVM: s390: Fix user triggerable bug in dead code
>       KVM: s390/mm: try a cow on read only pages for key ops
> 
>  arch/s390/kvm/kvm-s390.c | 13 -------------
>  arch/s390/mm/pgtable.c   | 10 ++++++++++
>  2 files changed, 10 insertions(+), 13 deletions(-)
> 

Thanks, pulled (for now locally).  Since the log message's about
malicious users, I assume there's no urgency in pushing this to Linus
and that normal guests work fine.

Paolo

Re: [GIT PULL 0/2] KVM: s390: Fixes for 3.17

[Christian Borntraeger]

On 25/08/14 15:42, Paolo Bonzini wrote:
> Il 25/08/2014 15:10, Christian Borntraeger ha scritto:
>> Paolo,
>>
>> the following changes since commit 7d1311b93e58ed55f3a31cc8f94c4b8fe988a2b9:
>>
>>   Linux 3.17-rc1 (2014-08-16 10:40:26 -0600)
>>
>> are available in the git repository at:
>>
>>   git://git.kernel.org/pub/scm/linux/kernel/git/kvms390/linux.git  tags/kvm-s390-20140825
>>
>> for you to fetch changes up to ab3f285f227fec62868037e9b1b1fd18294a83b8:
>>
>>   KVM: s390/mm: try a cow on read only pages for key ops (2014-08-25 14:35:28 +0200)
>>
>> ----------------------------------------------------------------
>> Here are two fixes for s390 KVM code that prevent:
>> 1. a malicious user to trigger a kernel BUG
>> 2. a malicious user to change the storage key of read-only pages
>>
>> ----------------------------------------------------------------
>> Christian Borntraeger (2):
>>       KVM: s390: Fix user triggerable bug in dead code
>>       KVM: s390/mm: try a cow on read only pages for key ops
>>
>>  arch/s390/kvm/kvm-s390.c | 13 -------------
>>  arch/s390/mm/pgtable.c   | 10 ++++++++++
>>  2 files changed, 10 insertions(+), 13 deletions(-)
>>
> 
> Thanks, pulled (for now locally).  Since the log message's about
> malicious users, I assume there's no urgency in pushing this to Linus
> and that normal guests work fine.

Yes.

Re: [GIT PULL 2/2] KVM: s390/mm: try a cow on read only pages for key ops

[Ben Hutchings]

On Mon, 2014-08-25 at 15:10 +0200, Christian Borntraeger wrote:
> The PFMF instruction handler  blindly wrote the storage key even if
> the page was mapped R/O in the host. Lets try a COW before continuing
> and bail out in case of errors.
> 
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Reviewed-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
> Cc: stable@vger.kernel.org
> ---
>  arch/s390/mm/pgtable.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
> index 19daa53..5404a62 100644
> --- a/arch/s390/mm/pgtable.c
> +++ b/arch/s390/mm/pgtable.c
> @@ -986,11 +986,21 @@ int set_guest_storage_key(struct mm_struct *mm, unsigned long addr,
>  	pte_t *ptep;
>  
>  	down_read(&mm->mmap_sem);
> +retry:
>  	ptep = get_locked_pte(current->mm, addr, &ptl);
>  	if (unlikely(!ptep)) {
>  		up_read(&mm->mmap_sem);
>  		return -EFAULT;
>  	}
> +	if (!(pte_val(*ptep) & _PAGE_INVALID) &&
> +	     (pte_val(*ptep) & _PAGE_PROTECT)) {
> +			pte_unmap_unlock(*ptep, ptl);
> +			if (fixup_user_fault(current, mm, addr, FAULT_FLAG_WRITE)) {
> +				up_read(&mm->mmap_sem);
> +				return -EFAULT;
> +			}
> +			goto retry;
> +		}

Every line below the first 'if' is indented one tab stop too far.

Ben.

>  	new = old = pgste_get_lock(ptep);
>  	pgste_val(new) &= ~(PGSTE_GR_BIT | PGSTE_GC_BIT |

-- 
Ben Hutchings
No political challenge can be met by shopping. - George Monbiot

Re: [GIT PULL 2/2] KVM: s390/mm: try a cow on read only pages for key ops

[Christian Borntraeger]

On 27/08/14 05:06, Ben Hutchings wrote:
> On Mon, 2014-08-25 at 15:10 +0200, Christian Borntraeger wrote:
>> The PFMF instruction handler  blindly wrote the storage key even if
>> the page was mapped R/O in the host. Lets try a COW before continuing
>> and bail out in case of errors.
>>
>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Reviewed-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
>> Cc: stable@vger.kernel.org
>> ---
>>  arch/s390/mm/pgtable.c | 10 ++++++++++
>>  1 file changed, 10 insertions(+)
>>
>> diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
>> index 19daa53..5404a62 100644
>> --- a/arch/s390/mm/pgtable.c
>> +++ b/arch/s390/mm/pgtable.c
>> @@ -986,11 +986,21 @@ int set_guest_storage_key(struct mm_struct *mm, unsigned long addr,
>>  	pte_t *ptep;
>>  
>>  	down_read(&mm->mmap_sem);
>> +retry:
>>  	ptep = get_locked_pte(current->mm, addr, &ptl);
>>  	if (unlikely(!ptep)) {
>>  		up_read(&mm->mmap_sem);
>>  		return -EFAULT;
>>  	}
>> +	if (!(pte_val(*ptep) & _PAGE_INVALID) &&
>> +	     (pte_val(*ptep) & _PAGE_PROTECT)) {
>> +			pte_unmap_unlock(*ptep, ptl);
>> +			if (fixup_user_fault(current, mm, addr, FAULT_FLAG_WRITE)) {
>> +				up_read(&mm->mmap_sem);
>> +				return -EFAULT;
>> +			}
>> +			goto retry;
>> +		}
> 
> Every line below the first 'if' is indented one tab stop too far.
> 
> Ben.
> 
>>  	new = old = pgste_get_lock(ptep);
>>  	pgste_val(new) &= ~(PGSTE_GR_BIT | PGSTE_GC_BIT |
> 

Hmm, indeed. Drat. Paolo, do you want a revert, resend?

Christian

Re: [GIT PULL 2/2] KVM: s390/mm: try a cow on read only pages for key ops

[Paolo Bonzini]

Il 27/08/2014 09:13, Christian Borntraeger ha scritto:
> On 27/08/14 05:06, Ben Hutchings wrote:
>> On Mon, 2014-08-25 at 15:10 +0200, Christian Borntraeger wrote:
>>> The PFMF instruction handler  blindly wrote the storage key even if
>>> the page was mapped R/O in the host. Lets try a COW before continuing
>>> and bail out in case of errors.
>>>
>>> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> Reviewed-by: Dominik Dingel <dingel@linux.vnet.ibm.com>
>>> Cc: stable@vger.kernel.org
>>> ---
>>>  arch/s390/mm/pgtable.c | 10 ++++++++++
>>>  1 file changed, 10 insertions(+)
>>>
>>> diff --git a/arch/s390/mm/pgtable.c b/arch/s390/mm/pgtable.c
>>> index 19daa53..5404a62 100644
>>> --- a/arch/s390/mm/pgtable.c
>>> +++ b/arch/s390/mm/pgtable.c
>>> @@ -986,11 +986,21 @@ int set_guest_storage_key(struct mm_struct *mm, unsigned long addr,
>>>  	pte_t *ptep;
>>>  
>>>  	down_read(&mm->mmap_sem);
>>> +retry:
>>>  	ptep = get_locked_pte(current->mm, addr, &ptl);
>>>  	if (unlikely(!ptep)) {
>>>  		up_read(&mm->mmap_sem);
>>>  		return -EFAULT;
>>>  	}
>>> +	if (!(pte_val(*ptep) & _PAGE_INVALID) &&
>>> +	     (pte_val(*ptep) & _PAGE_PROTECT)) {
>>> +			pte_unmap_unlock(*ptep, ptl);
>>> +			if (fixup_user_fault(current, mm, addr, FAULT_FLAG_WRITE)) {
>>> +				up_read(&mm->mmap_sem);
>>> +				return -EFAULT;
>>> +			}
>>> +			goto retry;
>>> +		}
>>
>> Every line below the first 'if' is indented one tab stop too far.
>>
>> Ben.
>>
>>>  	new = old = pgste_get_lock(ptep);
>>>  	pgste_val(new) &= ~(PGSTE_GR_BIT | PGSTE_GC_BIT |
>>
> 
> Hmm, indeed. Drat. Paolo, do you want a revert, resend?

Just send a trivial patch to fix up the formatting.

Paolo