Re: [PATCH] arm64: KVM: Fix user access for debug registers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Marc Zyngier <marc.zyngier@xxxxxxx> 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 <alex.bennee@xxxxxxxxxx>

>
> Reported-by: Peter Maydell <peter.maydell@xxxxxxxxxx>
> Cc: Alex Bennée <alex.bennee@xxxxxxxxxx>
> Fixes: 84e690bfbed1 ("KVM: arm64: introduce vcpu->arch.debug_ptr")
> Signed-off-by: Marc Zyngier <marc.zyngier@xxxxxxx>
> ---
>  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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux