Petr Tesarik e01e15
From: Thomas Huth <thuth@redhat.com>
Petr Tesarik e01e15
Date: Thu, 12 Sep 2019 13:54:38 +0200
Petr Tesarik e01e15
Subject: KVM: s390: Do not leak kernel stack data in the KVM_S390_INTERRUPT
Petr Tesarik e01e15
 ioctl
Petr Tesarik e01e15
Git-commit: 53936b5bf35e140ae27e4bbf0447a61063f400da
Petr Tesarik e01e15
Patch-mainline: v5.3
Petr Tesarik e01e15
References: git-fixes
Petr Tesarik e01e15
Petr Tesarik e01e15
When the userspace program runs the KVM_S390_INTERRUPT ioctl to inject
Petr Tesarik e01e15
an interrupt, we convert them from the legacy struct kvm_s390_interrupt
Petr Tesarik e01e15
to the new struct kvm_s390_irq via the s390int_to_s390irq() function.
Petr Tesarik e01e15
However, this function does not take care of all types of interrupts
Petr Tesarik e01e15
that we can inject into the guest later (see do_inject_vcpu()). Since we
Petr Tesarik e01e15
do not clear out the s390irq values before calling s390int_to_s390irq(),
Petr Tesarik e01e15
there is a chance that we copy random data from the kernel stack which
Petr Tesarik e01e15
could be leaked to the userspace later.
Petr Tesarik e01e15
Petr Tesarik e01e15
Specifically, the problem exists with the KVM_S390_INT_PFAULT_INIT
Petr Tesarik e01e15
interrupt: s390int_to_s390irq() does not handle it, and the function
Petr Tesarik e01e15
__inject_pfault_init() later copies irq->u.ext which contains the
Petr Tesarik e01e15
random kernel stack data. This data can then be leaked either to
Petr Tesarik e01e15
the guest memory in __deliver_pfault_init(), or the userspace might
Petr Tesarik e01e15
retrieve it directly with the KVM_S390_GET_IRQ_STATE ioctl.
Petr Tesarik e01e15
Petr Tesarik e01e15
Fix it by handling that interrupt type in s390int_to_s390irq(), too,
Petr Tesarik e01e15
and by making sure that the s390irq struct is properly pre-initialized.
Petr Tesarik e01e15
And while we're at it, make sure that s390int_to_s390irq() now
Petr Tesarik e01e15
directly returns -EINVAL for unknown interrupt types, so that we
Petr Tesarik e01e15
immediately get a proper error code in case we add more interrupt
Petr Tesarik e01e15
types to do_inject_vcpu() without updating s390int_to_s390irq()
Petr Tesarik e01e15
sometime in the future.
Petr Tesarik e01e15
Petr Tesarik e01e15
Cc: stable@vger.kernel.org
Petr Tesarik e01e15
Reviewed-by: David Hildenbrand <david@redhat.com>
Petr Tesarik e01e15
Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com>
Petr Tesarik e01e15
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Petr Tesarik e01e15
Signed-off-by: Thomas Huth <thuth@redhat.com>
Petr Tesarik e01e15
Link: https://lore.kernel.org/kvm/20190912115438.25761-1-thuth@redhat.com
Petr Tesarik e01e15
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
Petr Tesarik e01e15
Acked-by: Petr Tesarik <ptesarik@suse.com>
Petr Tesarik e01e15
---
Petr Tesarik e01e15
 arch/s390/kvm/interrupt.c |   10 ++++++++++
Petr Tesarik e01e15
 arch/s390/kvm/kvm-s390.c  |    2 +-
Petr Tesarik e01e15
 2 files changed, 11 insertions(+), 1 deletion(-)
Petr Tesarik e01e15
Petr Tesarik e01e15
--- a/arch/s390/kvm/interrupt.c
Petr Tesarik e01e15
+++ b/arch/s390/kvm/interrupt.c
Petr Tesarik e01e15
@@ -1714,6 +1714,16 @@ int s390int_to_s390irq(struct kvm_s390_i
Petr Tesarik e01e15
 	case KVM_S390_MCHK:
Petr Tesarik e01e15
 		irq->u.mchk.mcic = s390int->parm64;
Petr Tesarik e01e15
 		break;
Petr Tesarik e01e15
+	case KVM_S390_INT_PFAULT_INIT:
Petr Tesarik e01e15
+		irq->u.ext.ext_params = s390int->parm;
Petr Tesarik e01e15
+		irq->u.ext.ext_params2 = s390int->parm64;
Petr Tesarik e01e15
+		break;
Petr Tesarik e01e15
+	case KVM_S390_RESTART:
Petr Tesarik e01e15
+	case KVM_S390_INT_CLOCK_COMP:
Petr Tesarik e01e15
+	case KVM_S390_INT_CPU_TIMER:
Petr Tesarik e01e15
+		break;
Petr Tesarik e01e15
+	default:
Petr Tesarik e01e15
+		return -EINVAL;
Petr Tesarik e01e15
 	}
Petr Tesarik e01e15
 	return 0;
Petr Tesarik e01e15
 }
Petr Tesarik e01e15
--- a/arch/s390/kvm/kvm-s390.c
Petr Tesarik e01e15
+++ b/arch/s390/kvm/kvm-s390.c
Petr Tesarik e01e15
@@ -3775,7 +3775,7 @@ long kvm_arch_vcpu_ioctl(struct file *fi
Petr Tesarik e01e15
 	}
Petr Tesarik e01e15
 	case KVM_S390_INTERRUPT: {
Petr Tesarik e01e15
 		struct kvm_s390_interrupt s390int;
Petr Tesarik e01e15
-		struct kvm_s390_irq s390irq;
Petr Tesarik e01e15
+		struct kvm_s390_irq s390irq = {};
Petr Tesarik e01e15
 
Petr Tesarik e01e15
 		r = -EFAULT;
Petr Tesarik e01e15
 		if (copy_from_user(&s390int, argp, sizeof(s390int)))