2021-10-07 23:21:16

by Sean Christopherson

[permalink] [raw]
Subject: [PATCH] KVM: x86: Account for 32-bit kernels when handling address in TSC attrs

When handling TSC attributes, cast the userspace provided virtual address
to an unsigned long before casting it to a pointer to fix warnings on
32-bit kernels due to casting a 64-bit integer to a 32-bit pointer.

Add a check that the truncated address matches the original address, e.g.
to prevent userspace specifying garbage in bits 63:32.

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
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
4967 | u64 __user *uaddr = (u64 __user *)attr->addr;
| ^

Cc: Oliver Upton <[email protected]>
Fixes: 469fde25e680 ("KVM: x86: Expose TSC offset controls to userspace")
Signed-off-by: Sean Christopherson <[email protected]>
---
arch/x86/kvm/x86.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 196ac33ef958..4a52a08707de 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;
--
2.33.0.882.g93a45727a2-goog


2021-10-11 16:39:11

by Oliver Upton

[permalink] [raw]
Subject: Re: [PATCH] KVM: x86: Account for 32-bit kernels when handling address in TSC attrs

On Thu, Oct 7, 2021 at 6:16 PM Sean Christopherson <[email protected]> wrote:
>
> When handling TSC attributes, cast the userspace provided virtual address
> to an unsigned long before casting it to a pointer to fix warnings on
> 32-bit kernels due to casting a 64-bit integer to a 32-bit pointer.
>
> Add a check that the truncated address matches the original address, e.g.
> to prevent userspace specifying garbage in bits 63:32.
>
> 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
> 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
> 4967 | u64 __user *uaddr = (u64 __user *)attr->addr;
> | ^
>
> Cc: Oliver Upton <[email protected]>
> Fixes: 469fde25e680 ("KVM: x86: Expose TSC offset controls to userspace")
> Signed-off-by: Sean Christopherson <[email protected]>
> ---
> arch/x86/kvm/x86.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>

Reviewed-by: Oliver Upton <[email protected]>

2021-10-16 23:50:47

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH] KVM: x86: Account for 32-bit kernels when handling address in TSC attrs

On 11/10/21 16:35, Oliver Upton wrote:
> On Thu, Oct 7, 2021 at 6:16 PM Sean Christopherson <[email protected]> wrote:
>>
>> When handling TSC attributes, cast the userspace provided virtual address
>> to an unsigned long before casting it to a pointer to fix warnings on
>> 32-bit kernels due to casting a 64-bit integer to a 32-bit pointer.
>>
>> Add a check that the truncated address matches the original address, e.g.
>> to prevent userspace specifying garbage in bits 63:32.
>>
>> 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
>> 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
>> 4967 | u64 __user *uaddr = (u64 __user *)attr->addr;
>> | ^
>>
>> Cc: Oliver Upton <[email protected]>
>> Fixes: 469fde25e680 ("KVM: x86: Expose TSC offset controls to userspace")
>> Signed-off-by: Sean Christopherson <[email protected]>
>> ---
>> arch/x86/kvm/x86.c | 10 ++++++++--
>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>
>
> Reviewed-by: Oliver Upton <[email protected]>
>

Squashed, thanks.

Paolo