kvm_vm_release() closes vmp->fd and vmp->kvm_fd unconditionally, and
kvm_vm_free() calls kvm_vm_release() at teardown.  A test that calls
kvm_vm_release() and then kvm_vm_free() without a
vm_recreate_with_one_vcpu() in between double-closes both FDs.  Since
kvm_close() asserts on close() failure, the second close trips
TEST_ASSERT and aborts the test, or, if the FD was recycled, silently
closes an unrelated file.

Guard the two closes in kvm_vm_release() by checking each FD against
-1 and resetting it to -1 after closing, matching the existing
kvm_stats_release() idiom.  Existing in-tree callers all pass through
vm_recreate_with_one_vcpu() before teardown, so they reassign the FDs
and do not hit the bug today.

Fixes: fa3899add105 ("kvm: selftests: add basic test for state save and 
restore")
Signed-off-by: Fuad Tabba <[email protected]>
---
 tools/testing/selftests/kvm/lib/kvm_util.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/kvm/lib/kvm_util.c 
b/tools/testing/selftests/kvm/lib/kvm_util.c
index 2a76eca7029d..e44223714fd4 100644
--- a/tools/testing/selftests/kvm/lib/kvm_util.c
+++ b/tools/testing/selftests/kvm/lib/kvm_util.c
@@ -793,8 +793,14 @@ void kvm_vm_release(struct kvm_vm *vmp)
        list_for_each_entry_safe(vcpu, tmp, &vmp->vcpus, list)
                vm_vcpu_rm(vmp, vcpu);
 
-       kvm_close(vmp->fd);
-       kvm_close(vmp->kvm_fd);
+       if (vmp->fd >= 0) {
+               kvm_close(vmp->fd);
+               vmp->fd = -1;
+       }
+       if (vmp->kvm_fd >= 0) {
+               kvm_close(vmp->kvm_fd);
+               vmp->kvm_fd = -1;
+       }
 
        /* Free cached stats metadata and close FD */
        kvm_stats_release(&vmp->stats);
-- 
2.54.0.563.g4f69b47b94-goog


Reply via email to