* [PATCH] KVM: s390: Test for bad access register at the start of S390_MEM_OP
@ 2019-08-29 10:53 Thomas Huth
2019-08-29 11:15 ` Janosch Frank
2019-08-29 11:18 ` Cornelia Huck
0 siblings, 2 replies; 6+ messages in thread
From: Thomas Huth @ 2019-08-29 10:53 UTC (permalink / raw)
To: Christian Borntraeger, Janosch Frank, kvm
Cc: David Hildenbrand, Cornelia Huck, Heiko Carstens, Vasily Gorbik,
linux-s390, linux-kernel
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.
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
arch/s390/kvm/kvm-s390.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index f329dcb3f44c..725690853cbd 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -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)
--
2.18.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: s390: Test for bad access register at the start of S390_MEM_OP
2019-08-29 10:53 [PATCH] KVM: s390: Test for bad access register at the start of S390_MEM_OP Thomas Huth
@ 2019-08-29 11:15 ` Janosch Frank
2019-08-29 11:56 ` Thomas Huth
2019-08-29 11:18 ` Cornelia Huck
1 sibling, 1 reply; 6+ messages in thread
From: Janosch Frank @ 2019-08-29 11:15 UTC (permalink / raw)
To: Thomas Huth, Christian Borntraeger, kvm
Cc: David Hildenbrand, Cornelia Huck, Heiko Carstens, Vasily Gorbik,
linux-s390, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 1638 bytes --]
On 8/29/19 12:53 PM, Thomas Huth wrote:
> 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.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> arch/s390/kvm/kvm-s390.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index f329dcb3f44c..725690853cbd 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -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.
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: s390: Test for bad access register at the start of S390_MEM_OP
2019-08-29 10:53 [PATCH] KVM: s390: Test for bad access register at the start of S390_MEM_OP Thomas Huth
2019-08-29 11:15 ` Janosch Frank
@ 2019-08-29 11:18 ` Cornelia Huck
2019-08-29 11:47 ` Thomas Huth
1 sibling, 1 reply; 6+ messages in thread
From: Cornelia Huck @ 2019-08-29 11:18 UTC (permalink / raw)
To: Thomas Huth
Cc: Christian Borntraeger, Janosch Frank, kvm, David Hildenbrand,
Heiko Carstens, Vasily Gorbik, linux-s390, linux-kernel
On Thu, 29 Aug 2019 12:53:56 +0200
Thomas Huth <thuth@redhat.com> wrote:
> 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.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
> arch/s390/kvm/kvm-s390.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index f329dcb3f44c..725690853cbd 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -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.
>
> if (mop->size > MEM_OP_MAX_SIZE)
Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Btw: should Documentation/virt/kvm/api.txt spell out the valid range
for ar explicitly?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: s390: Test for bad access register at the start of S390_MEM_OP
2019-08-29 11:18 ` Cornelia Huck
@ 2019-08-29 11:47 ` Thomas Huth
2019-08-29 11:49 ` Cornelia Huck
0 siblings, 1 reply; 6+ messages in thread
From: Thomas Huth @ 2019-08-29 11:47 UTC (permalink / raw)
To: Cornelia Huck
Cc: Christian Borntraeger, Janosch Frank, kvm, David Hildenbrand,
Heiko Carstens, Vasily Gorbik, linux-s390, linux-kernel
On 29/08/2019 13.18, Cornelia Huck wrote:
[...]
>
> Btw: should Documentation/virt/kvm/api.txt spell out the valid range
> for ar explicitly?
>
That certainly would not hurt. Care to send a patch, or shall I assemble
one?
Thomas
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: s390: Test for bad access register at the start of S390_MEM_OP
2019-08-29 11:47 ` Thomas Huth
@ 2019-08-29 11:49 ` Cornelia Huck
0 siblings, 0 replies; 6+ messages in thread
From: Cornelia Huck @ 2019-08-29 11:49 UTC (permalink / raw)
To: Thomas Huth
Cc: Christian Borntraeger, Janosch Frank, kvm, David Hildenbrand,
Heiko Carstens, Vasily Gorbik, linux-s390, linux-kernel
On Thu, 29 Aug 2019 13:47:59 +0200
Thomas Huth <thuth@redhat.com> wrote:
> On 29/08/2019 13.18, Cornelia Huck wrote:
> [...]
> >
> > Btw: should Documentation/virt/kvm/api.txt spell 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.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] KVM: s390: Test for bad access register at the start of S390_MEM_OP
2019-08-29 11:15 ` Janosch Frank
@ 2019-08-29 11:56 ` Thomas Huth
0 siblings, 0 replies; 6+ messages in thread
From: Thomas Huth @ 2019-08-29 11:56 UTC (permalink / raw)
To: Janosch Frank, Christian Borntraeger, kvm
Cc: David Hildenbrand, Cornelia Huck, Heiko Carstens, Vasily Gorbik,
linux-s390, linux-kernel
[-- Attachment #1.1: Type: text/plain, Size: 319 bytes --]
On 29/08/2019 13.15, Janosch Frank wrote:
[...]
> By the way, I think we want to check mop->size for 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
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-08-29 11:56 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-29 10:53 [PATCH] KVM: s390: Test for bad access register at the start of S390_MEM_OP Thomas Huth
2019-08-29 11:15 ` Janosch Frank
2019-08-29 11:56 ` Thomas Huth
2019-08-29 11:18 ` Cornelia Huck
2019-08-29 11:47 ` Thomas Huth
2019-08-29 11:49 ` Cornelia Huck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).