[PATCH] s390/kvm: Reject memory region operations for ucontrol VMs

[PATCH] s390/kvm: Reject memory region operations for ucontrol VMs

[Christoph Schlameuss]

This change rejects the KVM_SET_USER_MEMORY_REGION and
KVM_SET_USER_MEMORY_REGION2 ioctls when called on a ucontrol VM.
This is neccessary since ucontrol VMs have kvm->arch.gmap set to 0 and
would thus result in a null pointer dereference further in.
Memory management needs to be performed in userspace and using the
ioctls KVM_S390_UCAS_MAP and KVM_S390_UCAS_UNMAP.

Also improve s390 specific documentation for KVM_SET_USER_MEMORY_REGION
and KVM_SET_USER_MEMORY_REGION2.

Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>
---
 Documentation/virt/kvm/api.rst | 12 ++++++++++++
 arch/s390/kvm/kvm-s390.c       |  3 +++
 2 files changed, 15 insertions(+)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index a71d91978d9e..eec8df1dde06 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -1403,6 +1403,12 @@ Instead, an abort (data abort if the cause of the page-table update
 was a load or a store, instruction abort if it was an instruction
 fetch) is injected in the guest.
 
+S390:
+^^^^^
+
+Returns -EINVAL if the VM has the KVM_VM_S390_UCONTROL flag set.
+Returns -EINVAL if called on a protected VM.
+
 4.36 KVM_SET_TSS_ADDR
 ---------------------
 
@@ -6273,6 +6279,12 @@ state.  At VM creation time, all memory is shared, i.e. the PRIVATE attribute
 is '0' for all gfns.  Userspace can control whether memory is shared/private by
 toggling KVM_MEMORY_ATTRIBUTE_PRIVATE via KVM_SET_MEMORY_ATTRIBUTES as needed.
 
+S390:
+^^^^^
+
+Returns -EINVAL if the VM has the KVM_VM_S390_UCONTROL flag set.
+Returns -EINVAL if called on a protected VM.
+
 4.141 KVM_SET_MEMORY_ATTRIBUTES
 -------------------------------
 
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 82e9631cd9ef..854d0d1410be 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -5748,6 +5748,9 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
 {
 	gpa_t size;
 
+	if (kvm_is_ucontrol(kvm))
+		return -EINVAL;
+
 	/* When we are protected, we should not change the memory slots */
 	if (kvm_s390_pv_get_handle(kvm))
 		return -EINVAL;

base-commit: f2661062f16b2de5d7b6a5c42a9a5c96326b8454
-- 
2.45.2

Re: [PATCH] s390/kvm: Reject memory region operations for ucontrol VMs

[Claudio Imbrenda]

On Mon, 24 Jun 2024 11:59:02 +0200
Christoph Schlameuss <schlameuss@linux.ibm.com> wrote:

> This change rejects the KVM_SET_USER_MEMORY_REGION and
> KVM_SET_USER_MEMORY_REGION2 ioctls when called on a ucontrol VM.
> This is neccessary since ucontrol VMs have kvm->arch.gmap set to 0 and
> would thus result in a null pointer dereference further in.
> Memory management needs to be performed in userspace and using the
> ioctls KVM_S390_UCAS_MAP and KVM_S390_UCAS_UNMAP.
> 
> Also improve s390 specific documentation for KVM_SET_USER_MEMORY_REGION
> and KVM_SET_USER_MEMORY_REGION2.
> 
> Signed-off-by: Christoph Schlameuss <schlameuss@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> ---
>  Documentation/virt/kvm/api.rst | 12 ++++++++++++
>  arch/s390/kvm/kvm-s390.c       |  3 +++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index a71d91978d9e..eec8df1dde06 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -1403,6 +1403,12 @@ Instead, an abort (data abort if the cause of the page-table update
>  was a load or a store, instruction abort if it was an instruction
>  fetch) is injected in the guest.
>  
> +S390:
> +^^^^^
> +
> +Returns -EINVAL if the VM has the KVM_VM_S390_UCONTROL flag set.
> +Returns -EINVAL if called on a protected VM.
> +
>  4.36 KVM_SET_TSS_ADDR
>  ---------------------
>  
> @@ -6273,6 +6279,12 @@ state.  At VM creation time, all memory is shared, i.e. the PRIVATE attribute
>  is '0' for all gfns.  Userspace can control whether memory is shared/private by
>  toggling KVM_MEMORY_ATTRIBUTE_PRIVATE via KVM_SET_MEMORY_ATTRIBUTES as needed.
>  
> +S390:
> +^^^^^
> +
> +Returns -EINVAL if the VM has the KVM_VM_S390_UCONTROL flag set.
> +Returns -EINVAL if called on a protected VM.
> +
>  4.141 KVM_SET_MEMORY_ATTRIBUTES
>  -------------------------------
>  
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 82e9631cd9ef..854d0d1410be 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -5748,6 +5748,9 @@ int kvm_arch_prepare_memory_region(struct kvm *kvm,
>  {
>  	gpa_t size;
>  
> +	if (kvm_is_ucontrol(kvm))
> +		return -EINVAL;
> +
>  	/* When we are protected, we should not change the memory slots */
>  	if (kvm_s390_pv_get_handle(kvm))
>  		return -EINVAL;
> 
> base-commit: f2661062f16b2de5d7b6a5c42a9a5c96326b8454

Re: [PATCH] s390/kvm: Reject memory region operations for ucontrol VMs

[Janosch Frank]

On 6/24/24 11:59, Christoph Schlameuss wrote:
> This change rejects the KVM_SET_USER_MEMORY_REGION and
> KVM_SET_USER_MEMORY_REGION2 ioctls when called on a ucontrol VM.
> This is neccessary since ucontrol VMs have kvm->arch.gmap set to 0 and
> would thus result in a null pointer dereference further in.
> Memory management needs to be performed in userspace and using the
> ioctls KVM_S390_UCAS_MAP and KVM_S390_UCAS_UNMAP.
> 
> Also improve s390 specific documentation for KVM_SET_USER_MEMORY_REGION
> and KVM_SET_USER_MEMORY_REGION2.
> 

I'll add this tag when picking since it's a fix:
Fixes: 27e0393f15fc ("KVM: s390: ucontrol: per vcpu address spaces")

It's a bit hard to track down the commit that should have contained your 
code to begin with and I think this is the closest we'll get.

Re: [PATCH] s390/kvm: Reject memory region operations for ucontrol VMs

[Paolo Bonzini]

On Mon, Jun 24, 2024 at 11:59 AM Christoph Schlameuss
<schlameuss@linux.ibm.com> wrote:
>
> This change rejects the KVM_SET_USER_MEMORY_REGION and
> KVM_SET_USER_MEMORY_REGION2 ioctls when called on a ucontrol VM.
> This is neccessary since ucontrol VMs have kvm->arch.gmap set to 0 and
> would thus result in a null pointer dereference further in.
> Memory management needs to be performed in userspace and using the
> ioctls KVM_S390_UCAS_MAP and KVM_S390_UCAS_UNMAP.
>
> Also improve s390 specific documentation for KVM_SET_USER_MEMORY_REGION
> and KVM_SET_USER_MEMORY_REGION2.

Would be nice to have a selftest for ucontrol VMs, too... just saying :)

Paolo

Re: [PATCH] s390/kvm: Reject memory region operations for ucontrol VMs

[Janosch Frank]

On 6/27/24 13:53, Paolo Bonzini wrote:
> On Mon, Jun 24, 2024 at 11:59 AM Christoph Schlameuss
> <schlameuss@linux.ibm.com> wrote:
>>
>> This change rejects the KVM_SET_USER_MEMORY_REGION and
>> KVM_SET_USER_MEMORY_REGION2 ioctls when called on a ucontrol VM.
>> This is neccessary since ucontrol VMs have kvm->arch.gmap set to 0 and
>> would thus result in a null pointer dereference further in.
>> Memory management needs to be performed in userspace and using the
>> ioctls KVM_S390_UCAS_MAP and KVM_S390_UCAS_UNMAP.
>>
>> Also improve s390 specific documentation for KVM_SET_USER_MEMORY_REGION
>> and KVM_SET_USER_MEMORY_REGION2.
> 
> Would be nice to have a selftest for ucontrol VMs, too... just saying :)
> 
> Paolo
> 

Already in the works, he just hasn't posted it yet :)
We did do a couple rounds of internal feedback on the tests first.

Re: [PATCH] s390/kvm: Reject memory region operations for ucontrol VMs

[Christoph Schlameuss]

On Thu, 27 Jun 2024 14:32:51 +0200
Janosch Frank <frankja@linux.ibm.com> wrote:

> On 6/27/24 13:53, Paolo Bonzini wrote:
> > On Mon, Jun 24, 2024 at 11:59 AM Christoph Schlameuss
> > <schlameuss@linux.ibm.com> wrote:  
> >>
> >> This change rejects the KVM_SET_USER_MEMORY_REGION and
> >> KVM_SET_USER_MEMORY_REGION2 ioctls when called on a ucontrol VM.
> >> This is neccessary since ucontrol VMs have kvm->arch.gmap set to 0 and
> >> would thus result in a null pointer dereference further in.
> >> Memory management needs to be performed in userspace and using the
> >> ioctls KVM_S390_UCAS_MAP and KVM_S390_UCAS_UNMAP.
> >>
> >> Also improve s390 specific documentation for KVM_SET_USER_MEMORY_REGION
> >> and KVM_SET_USER_MEMORY_REGION2.  
> > 
> > Would be nice to have a selftest for ucontrol VMs, too... just saying :)
> > 
> > Paolo
> >   
> 
> Already in the works, he just hasn't posted it yet :)
> We did do a couple rounds of internal feedback on the tests first.

I do also have a test case for this specifically, but it depends on the
base fixture. So I would send it together with that soon.

Christoph