@@ -512,18 +512,18 @@ int vgic_v3_parse_attr(struct kvm_device *dev, struct kvm_device_attr *attr,
*
* @dev: kvm device handle
* @attr: kvm device attribute
- * @reg: address the value is read or written
* @is_write: true if userspace is writing a register
*/
static int vgic_v3_attr_regs_access(struct kvm_device *dev,
struct kvm_device_attr *attr,
- u64 *reg, bool is_write)
+ bool is_write)
{
struct vgic_reg_attr reg_attr;
gpa_t addr;
struct kvm_vcpu *vcpu;
+ bool uaccess;
+ u32 val;
int ret;
- u32 tmp32;
ret = vgic_v3_parse_attr(dev, attr, ®_attr);
if (ret)
@@ -532,6 +532,21 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
vcpu = reg_attr.vcpu;
addr = reg_attr.addr;
+ switch (attr->group) {
+ case KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS:
+ /* Sysregs uaccess is performed by the sysreg handling code */
+ uaccess = false;
+ break;
+ default:
+ uaccess = true;
+ }
+
+ if (uaccess && is_write) {
+ u32 __user *uaddr = (u32 __user *)(unsigned long)attr->addr;
+ if (get_user(val, uaddr))
+ return -EFAULT;
+ }
+
mutex_lock(&dev->kvm->lock);
if (unlikely(!vgic_initialized(dev->kvm))) {
@@ -546,20 +561,10 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
switch (attr->group) {
case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
- if (is_write)
- tmp32 = *reg;
-
- ret = vgic_v3_dist_uaccess(vcpu, is_write, addr, &tmp32);
- if (!is_write)
- *reg = tmp32;
+ ret = vgic_v3_dist_uaccess(vcpu, is_write, addr, &val);
break;
case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
- if (is_write)
- tmp32 = *reg;
-
- ret = vgic_v3_redist_uaccess(vcpu, is_write, addr, &tmp32);
- if (!is_write)
- *reg = tmp32;
+ ret = vgic_v3_redist_uaccess(vcpu, is_write, addr, &val);
break;
case KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS:
ret = vgic_v3_cpu_sysregs_uaccess(vcpu, attr, is_write);
@@ -570,14 +575,10 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
info = (attr->attr & KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_MASK) >>
KVM_DEV_ARM_VGIC_LINE_LEVEL_INFO_SHIFT;
if (info == VGIC_LEVEL_INFO_LINE_LEVEL) {
- if (is_write)
- tmp32 = *reg;
intid = attr->attr &
KVM_DEV_ARM_VGIC_LINE_LEVEL_INTID_MASK;
ret = vgic_v3_line_level_info_uaccess(vcpu, is_write,
- intid, &tmp32);
- if (!is_write)
- *reg = tmp32;
+ intid, &val);
} else {
ret = -EINVAL;
}
@@ -591,6 +592,13 @@ static int vgic_v3_attr_regs_access(struct kvm_device *dev,
unlock_all_vcpus(dev->kvm);
out:
mutex_unlock(&dev->kvm->lock);
+
+ if (!ret && uaccess && !is_write) {
+ u32 __user *uaddr = (u32 __user *)(unsigned long)attr->addr;
+ if (put_user(val, uaddr))
+ ret = -EFAULT;
+ }
+
return ret;
}
@@ -605,30 +613,12 @@ static int vgic_v3_set_attr(struct kvm_device *dev,
switch (attr->group) {
case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
- case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS: {
- u32 __user *uaddr = (u32 __user *)(long)attr->addr;
- u32 tmp32;
- u64 reg;
-
- if (get_user(tmp32, uaddr))
- return -EFAULT;
-
- reg = tmp32;
- return vgic_v3_attr_regs_access(dev, attr, ®, true);
- }
+ case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
+ return vgic_v3_attr_regs_access(dev, attr, true);
case KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS:
- return vgic_v3_attr_regs_access(dev, attr, NULL, true);
- case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
- u32 __user *uaddr = (u32 __user *)(long)attr->addr;
- u64 reg;
- u32 tmp32;
-
- if (get_user(tmp32, uaddr))
- return -EFAULT;
-
- reg = tmp32;
- return vgic_v3_attr_regs_access(dev, attr, ®, true);
- }
+ return vgic_v3_attr_regs_access(dev, attr, true);
+ case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO:
+ return vgic_v3_attr_regs_access(dev, attr, true);
case KVM_DEV_ARM_VGIC_GRP_CTRL: {
int ret;
@@ -662,30 +652,12 @@ static int vgic_v3_get_attr(struct kvm_device *dev,
switch (attr->group) {
case KVM_DEV_ARM_VGIC_GRP_DIST_REGS:
- case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS: {
- u32 __user *uaddr = (u32 __user *)(long)attr->addr;
- u64 reg;
- u32 tmp32;
-
- ret = vgic_v3_attr_regs_access(dev, attr, ®, false);
- if (ret)
- return ret;
- tmp32 = reg;
- return put_user(tmp32, uaddr);
- }
+ case KVM_DEV_ARM_VGIC_GRP_REDIST_REGS:
+ return vgic_v3_attr_regs_access(dev, attr, false);
case KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS:
- return vgic_v3_attr_regs_access(dev, attr, NULL, false);
- case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO: {
- u32 __user *uaddr = (u32 __user *)(long)attr->addr;
- u64 reg;
- u32 tmp32;
-
- ret = vgic_v3_attr_regs_access(dev, attr, ®, false);
- if (ret)
- return ret;
- tmp32 = reg;
- return put_user(tmp32, uaddr);
- }
+ return vgic_v3_attr_regs_access(dev, attr, false);
+ case KVM_DEV_ARM_VGIC_GRP_LEVEL_INFO:
+ return vgic_v3_attr_regs_access(dev, attr, false);
}
return -ENXIO;
}
For userspace accesses to GICv3 MMIO registers (and related data), vgic_v3_{get,set}_attr are littered with {get,put}_user() calls, making it hard to audit and reason about. Consolidate all userspace accesses in vgic_v3_attr_regs_access(), makeing the code far simpler to audit. Signed-off-by: Marc Zyngier <maz@kernel.org> --- arch/arm64/kvm/vgic/vgic-kvm-device.c | 104 ++++++++++---------------- 1 file changed, 38 insertions(+), 66 deletions(-)