On 27/2/23 22:02, Dan Carpenter wrote:
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".
4460 mutex_unlock(&kvm->lock);
4461 if (ops->destroy)
--> 4462 ops->destroy(dev);
^^^
Use after free.
Nope, only one of these two callbacks is supposed to be defined - either
destroy() or release() but never both. Has it changed? It is not
extremely intuitive though. Thanks,
4463 return ret;
4464 }
4465
4466 cd->fd = ret;
4467 return 0;
4468 }
regards,
dan carpenter
--
Alexey
