On Thu, Sep 17, 2020 at 08:00:58PM +0800, Peng Liang wrote: > Currently, if we need to check the value of the register defined by user > space, we should check it in set_user. However, some system registers > may use the same set_user (for example, almost all ID registers), which > make it difficult to validate the value defined by user space.
If sharing set_user no longer makes sense for ID registers, then we need to rework the code so it's no longer shared. As I keep saying, we need to address this problem one ID register at a time. So, IMO, the approach should be to change one ID register at a time from using ID_SANITISED() to having its own table entry with its own set/get_user code. There may still be opportunity to share code among the ID registers, in which case refactoring can be done as needed too. Thanks, drew > > Introduce check_user to solve the problem. And apply check_user before > set_user to make sure that the value of register is valid. > > Signed-off-by: zhanghailiang <[email protected]> > Signed-off-by: Peng Liang <[email protected]> > --- > arch/arm64/kvm/sys_regs.c | 7 +++++++ > arch/arm64/kvm/sys_regs.h | 6 ++++++ > 2 files changed, 13 insertions(+) > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c > index 2b0fa8d5ac62..86ebb8093c3c 100644 > --- a/arch/arm64/kvm/sys_regs.c > +++ b/arch/arm64/kvm/sys_regs.c > @@ -2684,6 +2684,7 @@ int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, > const struct kvm_one_reg *reg > { > const struct sys_reg_desc *r; > void __user *uaddr = (void __user *)(unsigned long)reg->addr; > + int err; > > if ((reg->id & KVM_REG_ARM_COPROC_MASK) == KVM_REG_ARM_DEMUX) > return demux_c15_set(reg->id, uaddr); > @@ -2699,6 +2700,12 @@ int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, > const struct kvm_one_reg *reg > if (sysreg_hidden_from_user(vcpu, r)) > return -ENOENT; > > + if (r->check_user) { > + err = (r->check_user)(vcpu, r, reg, uaddr); > + if (err) > + return err; > + } > + > if (r->set_user) > return (r->set_user)(vcpu, r, reg, uaddr); > > diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h > index 5a6fc30f5989..9bce5e9a3490 100644 > --- a/arch/arm64/kvm/sys_regs.h > +++ b/arch/arm64/kvm/sys_regs.h > @@ -53,6 +53,12 @@ struct sys_reg_desc { > const struct kvm_one_reg *reg, void __user *uaddr); > int (*set_user)(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd, > const struct kvm_one_reg *reg, void __user *uaddr); > + /* > + * Check the value userspace passed. It should return 0 on success and > + * otherwise on failure. > + */ > + int (*check_user)(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd, > + const struct kvm_one_reg *reg, void __user *uaddr); > > /* Return mask of REG_* runtime visibility overrides */ > unsigned int (*visibility)(const struct kvm_vcpu *vcpu, > -- > 2.26.2 > _______________________________________________ kvmarm mailing list [email protected] https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
