On 08.11.17 12:09, Cornelia Huck wrote:
On Wed, 8 Nov 2017 12:04:22 +0100
Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote:
On 11/08/2017 10:19 AM, Cornelia Huck wrote:
On Wed, 8 Nov 2017 09:41:43 +0100
Christian Borntraeger <borntraeger@xxxxxxxxxx> wrote:
From: Michael Mueller <mimu@xxxxxxxxxxxxxxxxxx>
There is a chance to delete not yet delivered I/O interrupts if an
exploiter uses the subsystem identification word 0x0000 while
processing a KVM_DEV_FLIC_CLEAR_IO_IRQ ioctl. -EINVAL will be returned
now instead in that case.
Classic interrupts will always have bit 0x10000 set in the schid while
adapter interrupts have a zero schid. The clear_io_irq interface is
only useful for classic interrupts (as adapter interrupts belong to
many devices). Let's make this interface more strict and forbid a schid
of 0.
I'm wondering: Is there any possible use case to clear adapter
interrupts? This interface was introduced to handle the case where a
CRW was made pending for a subchannel (which implies any pending
interrupt needs to be cleared.)
The problem with clearing adapter interrupts is that is actually a summary
interrupt for every potential device. So I somewhat consider an adapter interrupt
pending when the summary indicator went from 0 to 1. So I dont think clearing
a single one makes not much sense. (And this interface would be wrong for
that I think)
Yes, this interface would be problematic. I'm not sure what's supposed
to happen with crws vs. pending adapter interrupts, though.
They will be delivered based on ISC priority and the traditional first
when both have the same class.
The only use cases I can imagine for clearing adapter interrupts (e.g. reset)
is already covered by KVM_DEV_FLIC_CLEAR_IRQS
Alas, I cannot check the adapter interrupt question myself, as the
public doc is lacking :( But qdio as another adapter interrupt user
comes to mind (not that we support it in qemu, but still...)
Signed-off-by: Michael Mueller <mimu@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Halil Pasic <pasic@xxxxxxxxxxxxxxxxxx>
Reviewed-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
---
Documentation/virtual/kvm/devices/s390_flic.txt | 3 +++
arch/s390/kvm/interrupt.c | 2 ++
2 files changed, 5 insertions(+)
diff --git a/Documentation/virtual/kvm/devices/s390_flic.txt b/Documentation/virtual/kvm/devices/s390_flic.txt
index 2f1cbf1..27ad53c 100644
--- a/Documentation/virtual/kvm/devices/s390_flic.txt
+++ b/Documentation/virtual/kvm/devices/s390_flic.txt
@@ -156,3 +156,6 @@ FLIC with an unknown group or attribute gives the error code EINVAL (instead of
ENXIO, as specified in the API documentation). It is not possible to conclude
that a FLIC operation is unavailable based on the error code resulting from a
usage attempt.
+
+Note: The KVM_DEV_FLIC_CLEAR_IO_IRQ ioctl will return EINVAL in case a zero
+schid is specified.
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index a3da4f3..c8aacce 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -2191,6 +2191,8 @@ static int clear_io_irq(struct kvm *kvm, struct kvm_device_attr *attr)
return -EINVAL;
if (copy_from_user(&schid, (void __user *) attr->addr, sizeof(schid)))
return -EFAULT;
+ if (!schid)
+ return -EINVAL;
kfree(kvm_s390_get_io_int(kvm, isc_mask, schid));
/*
* If userspace is conforming to the architecture, we can have at most
As this particular interface would not be a good match for whatever we
need to do for adapter interrupts, let's go with this patch.
Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>
