| Message ID | 20220402174044.2263418-2-oupton@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: arm64: Fix use-after-free in debugfs | expand |
On Sat, Apr 02, 2022 at 05:40:41PM +0000, Oliver Upton wrote: > Unfortunately, there is no guarantee that KVM was able to instantiate a > debugfs directory for a particular VM. To that end, KVM shouldn't even > attempt to create new debugfs files in this case. If the specified > parent dentry is NULL, debugfs_create_file() will instantiate files at > the root of debugfs. > > Since it is possible to create the vgic-state file outside of a VM > directory, the file is not cleaned up when a VM is destroyed. > Nonetheless, the corresponding struct kvm is freed when the VM is > destroyed. > > Plug the use-after-free by plainly refusing to create vgic-state when > KVM fails to create a VM debugfs dir. > > Cc: stable@kernel.org > Fixes: 929f45e32499 ("kvm: no need to check return value of debugfs_create functions") > Signed-off-by: Oliver Upton <oupton@google.com> Thinking about this a bit more... The game of whack-a-mole for other files that possibly have the same bug could probably be avoided if kvm->debugfs_dentry is initialized to PTR_ERR(-ENOENT) by default. That way there's no special error handling that needs to be done in KVM as any attempt to create a new file will bail. Going to test and send out a v2 most likely, just want to make sure no other use of debugfs in KVM will flip out from the change. -- Thanks, Oliver
diff --git a/arch/arm64/kvm/vgic/vgic-debug.c b/arch/arm64/kvm/vgic/vgic-debug.c index f38c40a76251..cf1364a6fabc 100644 --- a/arch/arm64/kvm/vgic/vgic-debug.c +++ b/arch/arm64/kvm/vgic/vgic-debug.c @@ -271,6 +271,9 @@ DEFINE_SEQ_ATTRIBUTE(vgic_debug); void vgic_debug_init(struct kvm *kvm) { + if (!kvm->debugfs_dentry) + return; + debugfs_create_file("vgic-state", 0444, kvm->debugfs_dentry, kvm, &vgic_debug_fops); }
Unfortunately, there is no guarantee that KVM was able to instantiate a debugfs directory for a particular VM. To that end, KVM shouldn't even attempt to create new debugfs files in this case. If the specified parent dentry is NULL, debugfs_create_file() will instantiate files at the root of debugfs. Since it is possible to create the vgic-state file outside of a VM directory, the file is not cleaned up when a VM is destroyed. Nonetheless, the corresponding struct kvm is freed when the VM is destroyed. Plug the use-after-free by plainly refusing to create vgic-state when KVM fails to create a VM debugfs dir. Cc: stable@kernel.org Fixes: 929f45e32499 ("kvm: no need to check return value of debugfs_create functions") Signed-off-by: Oliver Upton <oupton@google.com> --- arch/arm64/kvm/vgic/vgic-debug.c | 3 +++ 1 file changed, 3 insertions(+)