KVM: s390: Test for bad access register at the start of S390_MEM_OP
PATCH 1/1 KVM: s390: Test for bad access register at the start of S390_MEM_OP
If the KVM_S390_MEM_OP ioctl is called with an access register >= 16, then there is certainly a bug in the calling userspace application. We check for wrong access registers, but only if the vCPU was already in the access register mode before (i.e. the SIE block has recorded it). The check is also buried somewhere deep in the calling chain (in the function ar_translation()), so this is somewhat hard to find.
It's better to always report an error to the userspace in case this field is set wrong, and it's safer in the KVM code if we block wrong values here early instead of relying on a check somewhere deep down the calling chain, so let's add another check to kvm_s390_guest_mem_op() directly.
--- arch/s390/kvm/kvm-s390.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/s390/kvm/kvm-s390.c +1 -1
@@ -4255,7 +4255,7 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,const u64 supported_flags =KVM_S390_MEMOP_F_INJECT_EXCEPTION |KVM_S390_MEMOP_F_CHECK_ONLY ; -if (mop->flags & ~supported_flags) +if (mop->flags & ~supported_flags || mop->ar >=NUM_ACRS )return -EINVAL ;if (mop->size >MEM_OP_MAX_SIZE )
Signature · 1 line
2.18.1
Quoted 24 lines · Thomas Huth
If the
KVM_S390_MEM_OPioctl is called with an access register >= 16, then there is certainly a bug in the calling userspace application. We check for wrong access registers, but only if the vCPU was already in the access register mode before (i.e. the SIE block has recorded it). The check is also buried somewhere deep in the calling chain (in the functionar_translation()), so this is somewhat hard to find.It's better to always report an error to the userspace in case this field is set wrong, and it's safer in the KVM code if we block wrong values here early instead of relying on a check somewhere deep down the calling chain, so let's add another check to
kvm_s390_guest_mem_op()directly.Signed-off-by Thomas Huth <[email protected]>--- arch/s390/kvm/kvm-s390.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/s390/kvm/kvm-s390.c +0
-0
arch/s390/kvm/kvm-s390.c static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu, · line 4261
Quoted 8 lines
arch/s390/kvm/kvm-s390.c +1
-1@@ -4255,7 +4255,7 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,const u64 supported_flags =KVM_S390_MEMOP_F_INJECT_EXCEPTION |KVM_S390_MEMOP_F_CHECK_ONLY ; -if (mop->flags & ~supported_flags) +if (mop->flags & ~supported_flags || mop->ar >=NUM_ACRS )return -EINVAL ;if (mop->size >MEM_OP_MAX_SIZE )
By the way, I think we want to check mop->size for 0 before giving it to vmalloc and working with it.
On 29/08/2019 13.15, Janosch Frank wrote:
[...]
Quoted 2 lines
By the way, I think we want to check
mop->sizefor 0 before giving it to vmalloc and working with it.
You're right! This currently triggers a kernel warning message with a Call Trace! I'll add a check to my new memop selftest and send a patch...
Thomas
Quoted 23 lines · On Thu, 29 Aug 2019 12:53:56 +0200 Thomas Huth <[email protected]> wrote:
If the
KVM_S390_MEM_OPioctl is called with an access register >= 16, then there is certainly a bug in the calling userspace application. We check for wrong access registers, but only if the vCPU was already in the access register mode before (i.e. the SIE block has recorded it). The check is also buried somewhere deep in the calling chain (in the functionar_translation()), so this is somewhat hard to find.It's better to always report an error to the userspace in case this field is set wrong, and it's safer in the KVM code if we block wrong values here early instead of relying on a check somewhere deep down the calling chain, so let's add another check to
kvm_s390_guest_mem_op()directly.Signed-off-by Thomas Huth <[email protected]>--- arch/s390/kvm/kvm-s390.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)arch/s390/kvm/kvm-s390.c +0
-0
arch/s390/kvm/kvm-s390.c static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu, · line 4259
Quoted 6 lines
arch/s390/kvm/kvm-s390.c +1
-1@@ -4255,7 +4255,7 @@ static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu,const u64 supported_flags =KVM_S390_MEMOP_F_INJECT_EXCEPTION |KVM_S390_MEMOP_F_CHECK_ONLY ; -if (mop->flags & ~supported_flags) +if (mop->flags & ~supported_flags || mop->ar >=NUM_ACRS )return -EINVAL ;
This also matches the value that ar_translation would return, so seems sane.
arch/s390/kvm/kvm-s390.c static long kvm_s390_guest_mem_op(struct kvm_vcpu *vcpu, · line 4261
Quoted 2 lines
arch/s390/kvm/kvm-s390.c +0
-0if (mop->size >MEM_OP_MAX_SIZE )
Btw: should Documentation/virt/kvm/api.txt spell out the valid range for ar explicitly?
On 29/08/2019 13.18, Cornelia Huck wrote:
[...]
Quoted 4 lines
Btw: should
Documentation/virt/kvm/api.txtspell out the valid range for ar explicitly?
That certainly would not hurt. Care to send a patch, or shall I assemble one?
Thomas
Quoted 11 lines · On Thu, 29 Aug 2019 13:47:59 +0200 Thomas Huth <[email protected]> wrote:
On 29/08/2019 13.18, Cornelia Huck wrote:
[...]Quoted 4 lines
Btw: should
Documentation/virt/kvm/api.txtspell out the valid range for ar explicitly?That certainly would not hurt. Care to send a patch, or shall I assemble one?
Thomas
Will do.