| Message ID | 20220415201542.1496582-1-oupton@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | KVM: Clean up debugfs+stats init/destroy | expand |
+cc the actual kvmarm mailing list, what I had before was wishful thinking that we moved off of the list that always goes to my spam :-) On Fri, Apr 15, 2022 at 1:15 PM Oliver Upton <oupton@google.com> wrote: > > The way KVM handles debugfs initialization and destruction is somewhat > sloppy. Although the debugfs + stats bits get initialized *after* > kvm_create_vm(), they are torn down from kvm_destroy_vm(). And yes, > there is a window where we could theoretically destroy a VM before > debugfs is ever instantiated. > > This series does away with the mess by coupling debugfs+stats to the > overall VM create/destroy pattern. We already fail the VM creation if > kvm_create_vm_debugfs() fails, so there really isn't a need to do these > separately in the first place. > > The first two patches hoist some unrelated tidbits of stats state into > the debugfs constructors just so its all handled under one roof. > > The second two patches realize the the intention of the series, changing > the initialization order so we can get an FD for the vm early. > > Lastly, patch 5 is essentially a revert of Sean's proposed fix [1], but > I deliberately am not proposing a revert outright, in case alarm bells > go off that a stable patch got reverted (it is correct). > > Applies to the following commit w/ the addition of Sean's patch: > > fb649bda6f56 ("Merge tag 'block-5.18-2022-04-15' of git://git.kernel.dk/linux-block") > > Tested (I promise) on an Intel Skylake machine with KVM selftests. I > poked around in debugfs to make sure there were no stragglers, and I ran > the reproducer for [1] to confirm the null ptr deref wasn't introduced > yet again. > > Oliver Upton (5): > KVM: Shove vm stats_id init into kvm_create_vm_debugfs() > KVM: Shove vcpu stats_id init into kvm_vcpu_create_debugfs() > KVM: Get an fd before creating the VM > KVM: Actually create debugfs in kvm_create_vm() > KVM: Hoist debugfs_dentry init to kvm_create_vm_debugfs() (again) > > virt/kvm/kvm_main.c | 92 ++++++++++++++++++++++----------------------- > 1 file changed, 46 insertions(+), 46 deletions(-) > > -- > 2.36.0.rc0.470.gd361397f0d-goog >
The way KVM handles debugfs initialization and destruction is somewhat sloppy. Although the debugfs + stats bits get initialized *after* kvm_create_vm(), they are torn down from kvm_destroy_vm(). And yes, there is a window where we could theoretically destroy a VM before debugfs is ever instantiated. This series does away with the mess by coupling debugfs+stats to the overall VM create/destroy pattern. We already fail the VM creation if kvm_create_vm_debugfs() fails, so there really isn't a need to do these separately in the first place. The first two patches hoist some unrelated tidbits of stats state into the debugfs constructors just so its all handled under one roof. The second two patches realize the the intention of the series, changing the initialization order so we can get an FD for the vm early. Lastly, patch 5 is essentially a revert of Sean's proposed fix [1], but I deliberately am not proposing a revert outright, in case alarm bells go off that a stable patch got reverted (it is correct). Applies to the following commit w/ the addition of Sean's patch: fb649bda6f56 ("Merge tag 'block-5.18-2022-04-15' of git://git.kernel.dk/linux-block") Tested (I promise) on an Intel Skylake machine with KVM selftests. I poked around in debugfs to make sure there were no stragglers, and I ran the reproducer for [1] to confirm the null ptr deref wasn't introduced yet again. Oliver Upton (5): KVM: Shove vm stats_id init into kvm_create_vm_debugfs() KVM: Shove vcpu stats_id init into kvm_vcpu_create_debugfs() KVM: Get an fd before creating the VM KVM: Actually create debugfs in kvm_create_vm() KVM: Hoist debugfs_dentry init to kvm_create_vm_debugfs() (again) virt/kvm/kvm_main.c | 92 ++++++++++++++++++++++----------------------- 1 file changed, 46 insertions(+), 46 deletions(-)