Skip to content

Commit 00d8372

Browse files
Jens Freimannsashalevin
Jens Freimann
authored andcommitted
KVM: s390: fix get_all_floating_irqs
[ Upstream commit 94aa033 ] This fixes a bug introduced with commit c05c418 ("KVM: s390: add floating irq controller"). get_all_floating_irqs() does copy_to_user() while holding a spin lock. Let's fix this by filling a temporary buffer first and copy it to userspace after giving up the lock. Cc: <[email protected]> # 3.18+: 69a8d45 KVM: s390: no need to hold... Reviewed-by: David Hildenbrand <[email protected]> Signed-off-by: Jens Freimann <[email protected]> Signed-off-by: Christian Borntraeger <[email protected]> Acked-by: Cornelia Huck <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent 7e15bc0 commit 00d8372

File tree

2 files changed

+35
-26
lines changed

2 files changed

+35
-26
lines changed

Documentation/virtual/kvm/devices/s390_flic.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ Groups:
2727
Copies all floating interrupts into a buffer provided by userspace.
2828
When the buffer is too small it returns -ENOMEM, which is the indication
2929
for userspace to try again with a bigger buffer.
30+
-ENOBUFS is returned when the allocation of a kernelspace buffer has
31+
failed.
32+
-EFAULT is returned when copying data to userspace failed.
3033
All interrupts remain pending, i.e. are not deleted from the list of
3134
currently pending interrupts.
3235
attr->addr contains the userspace address of the buffer into which all

arch/s390/kvm/interrupt.c

Lines changed: 32 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <linux/mmu_context.h>
1717
#include <linux/signal.h>
1818
#include <linux/slab.h>
19+
#include <linux/vmalloc.h>
1920
#include <asm/asm-offsets.h>
2021
#include <asm/uaccess.h>
2122
#include "kvm-s390.h"
@@ -1036,61 +1037,66 @@ void kvm_s390_clear_float_irqs(struct kvm *kvm)
10361037
spin_unlock(&fi->lock);
10371038
}
10381039

1039-
static inline int copy_irq_to_user(struct kvm_s390_interrupt_info *inti,
1040-
u8 *addr)
1040+
static void inti_to_irq(struct kvm_s390_interrupt_info *inti,
1041+
struct kvm_s390_irq *irq)
10411042
{
1042-
struct kvm_s390_irq __user *uptr = (struct kvm_s390_irq __user *) addr;
1043-
struct kvm_s390_irq irq = {0};
1044-
1045-
irq.type = inti->type;
1043+
irq->type = inti->type;
10461044
switch (inti->type) {
10471045
case KVM_S390_INT_PFAULT_INIT:
10481046
case KVM_S390_INT_PFAULT_DONE:
10491047
case KVM_S390_INT_VIRTIO:
10501048
case KVM_S390_INT_SERVICE:
1051-
irq.u.ext = inti->ext;
1049+
irq->u.ext = inti->ext;
10521050
break;
10531051
case KVM_S390_INT_IO_MIN...KVM_S390_INT_IO_MAX:
1054-
irq.u.io = inti->io;
1052+
irq->u.io = inti->io;
10551053
break;
10561054
case KVM_S390_MCHK:
1057-
irq.u.mchk = inti->mchk;
1055+
irq->u.mchk = inti->mchk;
10581056
break;
1059-
default:
1060-
return -EINVAL;
10611057
}
1062-
1063-
if (copy_to_user(uptr, &irq, sizeof(irq)))
1064-
return -EFAULT;
1065-
1066-
return 0;
10671058
}
10681059

1069-
static int get_all_floating_irqs(struct kvm *kvm, __u8 *buf, __u64 len)
1060+
static int get_all_floating_irqs(struct kvm *kvm, u8 __user *usrbuf, u64 len)
10701061
{
10711062
struct kvm_s390_interrupt_info *inti;
10721063
struct kvm_s390_float_interrupt *fi;
1064+
struct kvm_s390_irq *buf;
1065+
int max_irqs;
10731066
int ret = 0;
10741067
int n = 0;
10751068

1069+
if (len > KVM_S390_FLIC_MAX_BUFFER || len == 0)
1070+
return -EINVAL;
1071+
1072+
/*
1073+
* We are already using -ENOMEM to signal
1074+
* userspace it may retry with a bigger buffer,
1075+
* so we need to use something else for this case
1076+
*/
1077+
buf = vzalloc(len);
1078+
if (!buf)
1079+
return -ENOBUFS;
1080+
1081+
max_irqs = len / sizeof(struct kvm_s390_irq);
1082+
10761083
fi = &kvm->arch.float_int;
10771084
spin_lock(&fi->lock);
1078-
10791085
list_for_each_entry(inti, &fi->list, list) {
1080-
if (len < sizeof(struct kvm_s390_irq)) {
1086+
if (n == max_irqs) {
10811087
/* signal userspace to try again */
10821088
ret = -ENOMEM;
10831089
break;
10841090
}
1085-
ret = copy_irq_to_user(inti, buf);
1086-
if (ret)
1087-
break;
1088-
buf += sizeof(struct kvm_s390_irq);
1089-
len -= sizeof(struct kvm_s390_irq);
1091+
inti_to_irq(inti, &buf[n]);
10901092
n++;
10911093
}
1092-
10931094
spin_unlock(&fi->lock);
1095+
if (!ret && n > 0) {
1096+
if (copy_to_user(usrbuf, buf, sizeof(struct kvm_s390_irq) * n))
1097+
ret = -EFAULT;
1098+
}
1099+
vfree(buf);
10941100

10951101
return ret < 0 ? ret : n;
10961102
}
@@ -1101,7 +1107,7 @@ static int flic_get_attr(struct kvm_device *dev, struct kvm_device_attr *attr)
11011107

11021108
switch (attr->group) {
11031109
case KVM_DEV_FLIC_GET_ALL_IRQS:
1104-
r = get_all_floating_irqs(dev->kvm, (u8 *) attr->addr,
1110+
r = get_all_floating_irqs(dev->kvm, (u8 __user *) attr->addr,
11051111
attr->attr);
11061112
break;
11071113
default:

0 commit comments

Comments
 (0)