> From: Dan Carpenter <error27@xxxxxxxxx>
> Sent: Monday, February 27, 2023 7:02 PM
>
> Hello Alexey Kardashevskiy,
>
> The patch e8bc24270188: "KVM: Don't null dereference ops->destroy"
> from Jun 1, 2022, leads to the following Smatch static checker
> warning:
>
> arch/x86/kvm/../../../virt/kvm/kvm_main.c:4462
> kvm_ioctl_create_device()
> warn: 'dev' was already freed.
>
> arch/x86/kvm/../../../virt/kvm/kvm_main.c
> 4449 if (ops->init)
> 4450 ops->init(dev);
> 4451
> 4452 kvm_get_kvm(kvm);
> 4453 ret = anon_inode_getfd(ops->name, &kvm_device_fops, dev,
> O_RDWR | O_CLOEXEC);
> 4454 if (ret < 0) {
> 4455 kvm_put_kvm_no_destroy(kvm);
> 4456 mutex_lock(&kvm->lock);
> 4457 list_del(&dev->vm_node);
> 4458 if (ops->release)
> 4459 ops->release(dev);
> ^^^
> The kvm_vfio_release() frees "dev".
It appears that release op and destroy op will not co-exist. So this
use after free may not happen. Is it?
/*
* Release is an alternative method to free the device. It is
* called when the device file descriptor is closed. Once
* release is called, the destroy method will not be called
* anymore as the device is removed from the device list of
* the VM. kvm->lock is held.
*/
>
> 4460 mutex_unlock(&kvm->lock);
> 4461 if (ops->destroy)
> --> 4462 ops->destroy(dev);
> ^^^
> Use after free.
>
> 4463 return ret;
> 4464 }
> 4465
> 4466 cd->fd = ret;
> 4467 return 0;
> 4468 }
>
> regards,
> dan carpenter
Regards,
Yi Liu
