Commit e1246f3f authored by Marc Zyngier's avatar Marc Zyngier
Browse files

KVM: arm64: vgic-v3: Consolidate userspace access for MMIO registers 



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(),
making the code far simpler to audit.

Reviewed-by: default avatarReiji Watanabe <reijiw@google.com>
Signed-off-by: default avatarMarc Zyngier <maz@kernel.org>
parent 38cf0bb7

arch/arm64/kvm/vgic/vgic-kvm-device.c

+37 −66
Original line number Diff line number Diff line
@@ -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, NULL for sysregs

 * @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, &reg_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,12 @@ 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;

		ret = put_user(val, uaddr);

	}



	return ret;

}
@@ -605,30 +612,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, &reg, 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, &reg, 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 +651,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, &reg, 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, &reg, 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;

}