[bug report] KVM: Introduce a 'release' method for KVM devices

[bug report] KVM: Introduce a 'release' method for KVM devices

[Dan Carpenter]

Hello Cédric Le Goater,

This is a semi-automatic email about new static checker warnings.

The patch 2bde9b3ec8bd: "KVM: Introduce a 'release' method for KVM 
devices" from Apr 18, 2019, leads to the following Smatch complaint:

    arch/x86/kvm/../../../virt/kvm/kvm_main.c:2943 kvm_device_release()
    warn: variable dereferenced before check 'dev' (see line 2941)

arch/x86/kvm/../../../virt/kvm/kvm_main.c
  2938  static int kvm_device_release(struct inode *inode, struct file *filp)
  2939  {
  2940		struct kvm_device *dev = filp->private_data;
  2941		struct kvm *kvm = dev->kvm;
                                  ^^^^^^^^
Dereference.

  2942	
  2943		if (!dev)
                    ^^^^
Checked too late.

  2944			return -ENODEV;
  2945	
  2946          if (dev->kvm != kvm)
                    ^^^^^^^^^^^^^^^
What is this testing?  We just set "kvm = dev->kvm;" at the start.

  2947                  return -EPERM;
  2948  
  2949          if (dev->ops->release) {
  2950                  mutex_lock(&kvm->lock);
  2951                  list_del(&dev->vm_node);
  2952                  dev->ops->release(dev);
  2953                  mutex_unlock(&kvm->lock);
  2954          }
  2955  
  2956          kvm_put_kvm(kvm);
  2957          return 0;
  2958  }

regards,
dan carpenter

Re: [bug report] KVM: Introduce a 'release' method for KVM devices

[Alexey Kardashevskiy]

On 02/05/2019 17:03, Dan Carpenter wrote:
> Hello Cédric Le Goater,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 2bde9b3ec8bd: "KVM: Introduce a 'release' method for KVM 
> devices" from Apr 18, 2019, leads to the following Smatch complaint:


Already reported https://lkml.org/lkml/2019/5/1/235


> 
>     arch/x86/kvm/../../../virt/kvm/kvm_main.c:2943 kvm_device_release()
>     warn: variable dereferenced before check 'dev' (see line 2941)
> 
> arch/x86/kvm/../../../virt/kvm/kvm_main.c
>   2938  static int kvm_device_release(struct inode *inode, struct file *filp)
>   2939  {
>   2940		struct kvm_device *dev = filp->private_data;
>   2941		struct kvm *kvm = dev->kvm;
>                                   ^^^^^^^^
> Dereference.
> 
>   2942	
>   2943		if (!dev)
>                     ^^^^
> Checked too late.
> 
>   2944			return -ENODEV;
>   2945	
>   2946          if (dev->kvm != kvm)
>                     ^^^^^^^^^^^^^^^
> What is this testing?  We just set "kvm = dev->kvm;" at the start.
> 
>   2947                  return -EPERM;
>   2948  
>   2949          if (dev->ops->release) {
>   2950                  mutex_lock(&kvm->lock);
>   2951                  list_del(&dev->vm_node);
>   2952                  dev->ops->release(dev);
>   2953                  mutex_unlock(&kvm->lock);
>   2954          }
>   2955  
>   2956          kvm_put_kvm(kvm);
>   2957          return 0;
>   2958  }
> 
> regards,
> dan carpenter
> 

-- 
Alexey