Re: [PATCH v8 7/7] KVM: x86: Expose TSC offset controls to userspace

[Sean Christopherson <seanjc@xxxxxxxxxx>]

On Thu, Sep 16, 2021, Oliver Upton wrote:
> +static int kvm_arch_tsc_get_attr(struct kvm_vcpu *vcpu,
> +				 struct kvm_device_attr *attr)
> +{
> +	u64 __user *uaddr = (u64 __user *)attr->addr;

...

> +static int kvm_arch_tsc_set_attr(struct kvm_vcpu *vcpu,
> +				 struct kvm_device_attr *attr)
> +{
> +	u64 __user *uaddr = (u64 __user *)attr->addr;

These casts break 32-bit builds because of truncating attr->addr from 64-bit int
to a 32-bit pointer.  The address should also be checked to verify bits 63:32 are
not set on 32-bit kernels.

arch/x86/kvm/x86.c: In function ‘kvm_arch_tsc_get_attr’:
arch/x86/kvm/x86.c:4947:22: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
 4947 |  u64 __user *uaddr = (u64 __user *)attr->addr;
      |                      ^
arch/x86/kvm/x86.c: In function ‘kvm_arch_tsc_set_attr’:
arch/x86/kvm/x86.c:4967:22: error: cast to pointer from integer of different size [-Werror=int-to-pointer-cast]
 4967 |  u64 __user *uaddr = (u64 __user *)attr->addr;
      |                      ^


Not sure if there's a more elegant approach than casts galore?

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 8e5e462ffd65..3930e5dcdf0e 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -4944,9 +4944,12 @@ static int kvm_arch_tsc_has_attr(struct kvm_vcpu *vcpu,
 static int kvm_arch_tsc_get_attr(struct kvm_vcpu *vcpu,
                                 struct kvm_device_attr *attr)
 {
-       u64 __user *uaddr = (u64 __user *)attr->addr;
+       u64 __user *uaddr = (u64 __user *)(unsigned long)attr->addr;
        int r;

+       if ((u64)(unsigned long)uaddr != attr->addr)
+               return -EFAULT;
+
        switch (attr->attr) {
        case KVM_VCPU_TSC_OFFSET:
                r = -EFAULT;
@@ -4964,10 +4967,13 @@ static int kvm_arch_tsc_get_attr(struct kvm_vcpu *vcpu,
 static int kvm_arch_tsc_set_attr(struct kvm_vcpu *vcpu,
                                 struct kvm_device_attr *attr)
 {
-       u64 __user *uaddr = (u64 __user *)attr->addr;
+       u64 __user *uaddr = (u64 __user *)(unsigned long)attr->addr;
        struct kvm *kvm = vcpu->kvm;
        int r;

+       if ((u64)(unsigned long)uaddr != attr->addr)
+               return -EFAULT;
+
        switch (attr->attr) {
        case KVM_VCPU_TSC_OFFSET: {
                u64 offset, tsc, ns;