Marc Zyngier <[email protected]> writes:

> When setting the debug register from userspace, make sure that
> copy_from_user() is called with its parameters in the expected
> order. It otherwise doesn't do what you think.

Oops. Well that exposes a big hole in my testing. While I tested
debugging inside the guest worked before and after being guest debugged
I think GDBs tendency to reload all the debug registers between each
step may have masked this.

Debugging GDB in action or some sort of migration event would of course
screw this up but I'm afraid my testing wasn't evil enough.

Anyway have a:

Reviewed-by: Alex Bennée <[email protected]>

>
> Reported-by: Peter Maydell <[email protected]>
> Cc: Alex Bennée <[email protected]>
> Fixes: 84e690bfbed1 ("KVM: arm64: introduce vcpu->arch.debug_ptr")
> Signed-off-by: Marc Zyngier <[email protected]>
> ---
>  arch/arm64/kvm/sys_regs.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index b41607d..1d0463e 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -272,7 +272,7 @@ static int set_bvr(struct kvm_vcpu *vcpu, const struct 
> sys_reg_desc *rd,
>  {
>       __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_bvr[rd->reg];
>  
> -     if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> +     if (copy_from_user(r, uaddr, KVM_REG_SIZE(reg->id)) != 0)
>               return -EFAULT;
>       return 0;
>  }
> @@ -314,7 +314,7 @@ static int set_bcr(struct kvm_vcpu *vcpu, const struct 
> sys_reg_desc *rd,
>  {
>       __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_bcr[rd->reg];
>  
> -     if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> +     if (copy_from_user(r, uaddr, KVM_REG_SIZE(reg->id)) != 0)
>               return -EFAULT;
>  
>       return 0;
> @@ -358,7 +358,7 @@ static int set_wvr(struct kvm_vcpu *vcpu, const struct 
> sys_reg_desc *rd,
>  {
>       __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_wvr[rd->reg];
>  
> -     if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> +     if (copy_from_user(r, uaddr, KVM_REG_SIZE(reg->id)) != 0)
>               return -EFAULT;
>       return 0;
>  }
> @@ -400,7 +400,7 @@ static int set_wcr(struct kvm_vcpu *vcpu, const struct 
> sys_reg_desc *rd,
>  {
>       __u64 *r = &vcpu->arch.vcpu_debug_state.dbg_wcr[rd->reg];
>  
> -     if (copy_from_user(uaddr, r, KVM_REG_SIZE(reg->id)) != 0)
> +     if (copy_from_user(r, uaddr, KVM_REG_SIZE(reg->id)) != 0)
>               return -EFAULT;
>       return 0;
>  }

-- 
Alex Bennée
_______________________________________________
kvmarm mailing list
[email protected]
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Reply via email to