Skip to contentSkip to conversation

KVM: Introduce a 'release' method for KVM devices

bug report Dan Carpenter 2 participants 2 messages May 2, 2019 KVM

lore
Share & tools
lore
#0 lore raw

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

#1 lore raw
Quoted 6 lines · 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:

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

Quoted 39 lines

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

Signature · 1 line
Alexey