From: Sean Christopherson <seanjc@google.com>
To: kbuild-all@lists.01.org
Subject: Re: [kbuild] arch/x86/kvm/x86.c:4988 kvm_arch_tsc_set_attr() warn: check for integer overflow 'offset'
Date: Mon, 10 Oct 2022 18:39:57 +0000 [thread overview]
Message-ID: <Y0Rm/Y5flhd734NX@google.com> (raw)
In-Reply-To: <202210102159.8nYEC0Hl-lkp@intel.com>
[-- Attachment #1: Type: text/plain, Size: 3069 bytes --]
On Mon, Oct 10, 2022, Dan Carpenter wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 493ffd6605b2d3d4dc7008ab927dba319f36671f
> commit: 828ca89628bfcb1b8f27535025f69dd00eb55207 KVM: x86: Expose TSC offset controls to userspace
> config: x86_64-randconfig-m001-20221010
> compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> smatch warnings:
> arch/x86/kvm/x86.c:4988 kvm_arch_tsc_set_attr() warn: check for integer overflow 'offset'
>
> vim +/offset +4988 arch/x86/kvm/x86.c
>
> 828ca89628bfcb Oliver Upton 2021-09-16 4962 static int kvm_arch_tsc_set_attr(struct kvm_vcpu *vcpu,
> 828ca89628bfcb Oliver Upton 2021-09-16 4963 struct kvm_device_attr *attr)
> 828ca89628bfcb Oliver Upton 2021-09-16 4964 {
> 828ca89628bfcb Oliver Upton 2021-09-16 4965 u64 __user *uaddr = (u64 __user *)(unsigned long)attr->addr;
> 828ca89628bfcb Oliver Upton 2021-09-16 4966 struct kvm *kvm = vcpu->kvm;
> 828ca89628bfcb Oliver Upton 2021-09-16 4967 int r;
> 828ca89628bfcb Oliver Upton 2021-09-16 4968
> 828ca89628bfcb Oliver Upton 2021-09-16 4969 if ((u64)(unsigned long)uaddr != attr->addr)
> 828ca89628bfcb Oliver Upton 2021-09-16 4970 return -EFAULT;
> 828ca89628bfcb Oliver Upton 2021-09-16 4971
> 828ca89628bfcb Oliver Upton 2021-09-16 4972 switch (attr->attr) {
> 828ca89628bfcb Oliver Upton 2021-09-16 4973 case KVM_VCPU_TSC_OFFSET: {
> 828ca89628bfcb Oliver Upton 2021-09-16 4974 u64 offset, tsc, ns;
> 828ca89628bfcb Oliver Upton 2021-09-16 4975 unsigned long flags;
> 828ca89628bfcb Oliver Upton 2021-09-16 4976 bool matched;
> 828ca89628bfcb Oliver Upton 2021-09-16 4977
> 828ca89628bfcb Oliver Upton 2021-09-16 4978 r = -EFAULT;
> 828ca89628bfcb Oliver Upton 2021-09-16 4979 if (get_user(offset, uaddr))
> 828ca89628bfcb Oliver Upton 2021-09-16 4980 break;
> 828ca89628bfcb Oliver Upton 2021-09-16 4981
> 828ca89628bfcb Oliver Upton 2021-09-16 4982 raw_spin_lock_irqsave(&kvm->arch.tsc_write_lock, flags);
> 828ca89628bfcb Oliver Upton 2021-09-16 4983
> 828ca89628bfcb Oliver Upton 2021-09-16 4984 matched = (vcpu->arch.virtual_tsc_khz &&
> 828ca89628bfcb Oliver Upton 2021-09-16 4985 kvm->arch.last_tsc_khz == vcpu->arch.virtual_tsc_khz &&
> 828ca89628bfcb Oliver Upton 2021-09-16 4986 kvm->arch.last_tsc_offset == offset);
> 828ca89628bfcb Oliver Upton 2021-09-16 4987
> 828ca89628bfcb Oliver Upton 2021-09-16 @4988 tsc = kvm_scale_tsc(vcpu, rdtsc(), vcpu->arch.l1_tsc_scaling_ratio) + offset;
>
> Smatch hates obvious user triggerable integer overflows... No checking
> on offset.
This is ok, and even necessary, e.g. if the host TSC > guest TSC. Is there anything
we can do in KVM to help Smatch avoid false positives? Or do you/Smatch already
maintain a list of known false positives?
WARNING: multiple messages have this Message-ID (diff)
From: Sean Christopherson <seanjc@google.com>
To: Dan Carpenter <dan.carpenter@oracle.com>
Cc: kbuild@lists.01.org, Oliver Upton <oupton@google.com>,
lkp@intel.com, kbuild-all@lists.01.org,
linux-kernel@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [kbuild] arch/x86/kvm/x86.c:4988 kvm_arch_tsc_set_attr() warn: check for integer overflow 'offset'
Date: Mon, 10 Oct 2022 18:39:57 +0000 [thread overview]
Message-ID: <Y0Rm/Y5flhd734NX@google.com> (raw)
In-Reply-To: <202210102159.8nYEC0Hl-lkp@intel.com>
On Mon, Oct 10, 2022, Dan Carpenter wrote:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 493ffd6605b2d3d4dc7008ab927dba319f36671f
> commit: 828ca89628bfcb1b8f27535025f69dd00eb55207 KVM: x86: Expose TSC offset controls to userspace
> config: x86_64-randconfig-m001-20221010
> compiler: gcc-11 (Debian 11.3.0-5) 11.3.0
>
> If you fix the issue, kindly add following tag where applicable
> | Reported-by: kernel test robot <lkp@intel.com>
> | Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> smatch warnings:
> arch/x86/kvm/x86.c:4988 kvm_arch_tsc_set_attr() warn: check for integer overflow 'offset'
>
> vim +/offset +4988 arch/x86/kvm/x86.c
>
> 828ca89628bfcb Oliver Upton 2021-09-16 4962 static int kvm_arch_tsc_set_attr(struct kvm_vcpu *vcpu,
> 828ca89628bfcb Oliver Upton 2021-09-16 4963 struct kvm_device_attr *attr)
> 828ca89628bfcb Oliver Upton 2021-09-16 4964 {
> 828ca89628bfcb Oliver Upton 2021-09-16 4965 u64 __user *uaddr = (u64 __user *)(unsigned long)attr->addr;
> 828ca89628bfcb Oliver Upton 2021-09-16 4966 struct kvm *kvm = vcpu->kvm;
> 828ca89628bfcb Oliver Upton 2021-09-16 4967 int r;
> 828ca89628bfcb Oliver Upton 2021-09-16 4968
> 828ca89628bfcb Oliver Upton 2021-09-16 4969 if ((u64)(unsigned long)uaddr != attr->addr)
> 828ca89628bfcb Oliver Upton 2021-09-16 4970 return -EFAULT;
> 828ca89628bfcb Oliver Upton 2021-09-16 4971
> 828ca89628bfcb Oliver Upton 2021-09-16 4972 switch (attr->attr) {
> 828ca89628bfcb Oliver Upton 2021-09-16 4973 case KVM_VCPU_TSC_OFFSET: {
> 828ca89628bfcb Oliver Upton 2021-09-16 4974 u64 offset, tsc, ns;
> 828ca89628bfcb Oliver Upton 2021-09-16 4975 unsigned long flags;
> 828ca89628bfcb Oliver Upton 2021-09-16 4976 bool matched;
> 828ca89628bfcb Oliver Upton 2021-09-16 4977
> 828ca89628bfcb Oliver Upton 2021-09-16 4978 r = -EFAULT;
> 828ca89628bfcb Oliver Upton 2021-09-16 4979 if (get_user(offset, uaddr))
> 828ca89628bfcb Oliver Upton 2021-09-16 4980 break;
> 828ca89628bfcb Oliver Upton 2021-09-16 4981
> 828ca89628bfcb Oliver Upton 2021-09-16 4982 raw_spin_lock_irqsave(&kvm->arch.tsc_write_lock, flags);
> 828ca89628bfcb Oliver Upton 2021-09-16 4983
> 828ca89628bfcb Oliver Upton 2021-09-16 4984 matched = (vcpu->arch.virtual_tsc_khz &&
> 828ca89628bfcb Oliver Upton 2021-09-16 4985 kvm->arch.last_tsc_khz == vcpu->arch.virtual_tsc_khz &&
> 828ca89628bfcb Oliver Upton 2021-09-16 4986 kvm->arch.last_tsc_offset == offset);
> 828ca89628bfcb Oliver Upton 2021-09-16 4987
> 828ca89628bfcb Oliver Upton 2021-09-16 @4988 tsc = kvm_scale_tsc(vcpu, rdtsc(), vcpu->arch.l1_tsc_scaling_ratio) + offset;
>
> Smatch hates obvious user triggerable integer overflows... No checking
> on offset.
This is ok, and even necessary, e.g. if the host TSC > guest TSC. Is there anything
we can do in KVM to help Smatch avoid false positives? Or do you/Smatch already
maintain a list of known false positives?
next prev parent reply other threads:[~2022-10-10 18:39 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-10 13:56 arch/x86/kvm/x86.c:4988 kvm_arch_tsc_set_attr() warn: check for integer overflow 'offset' kernel test robot
2022-10-10 14:15 ` [kbuild] " Dan Carpenter
2022-10-10 14:15 ` Dan Carpenter
2022-10-10 18:39 ` Sean Christopherson [this message]
2022-10-10 18:39 ` Sean Christopherson
2022-10-11 13:02 ` Paolo Bonzini
2022-10-11 13:02 ` Paolo Bonzini
2022-10-12 6:34 ` Dan Carpenter
2022-10-12 6:34 ` [kbuild] " Dan Carpenter
2022-10-12 6:34 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y0Rm/Y5flhd734NX@google.com \
--to=seanjc@google.com \
--cc=kbuild-all@lists.01.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.