| Message ID | 20240624095902.29375-1-schlameuss@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390/kvm: Reject memory region operations for ucontrol VMs | expand |
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
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.
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
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.
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
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;
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(+) base-commit: f2661062f16b2de5d7b6a5c42a9a5c96326b8454