Patch "KVM: PPC: Book3S HV: Prevent UAF in kvm_spapr_tce_attach_iommu_group()" has been added to the 6.9-stable tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



This is a note to let you know that I've just added the patch titled

    KVM: PPC: Book3S HV: Prevent UAF in kvm_spapr_tce_attach_iommu_group()

to the 6.9-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     kvm-ppc-book3s-hv-prevent-uaf-in-kvm_spapr_tce_attac.patch
and it can be found in the queue-6.9 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.



commit 32d8cdfb739c44146020530527cf7e1cbc7dfbaa
Author: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
Date:   Fri Jun 14 22:29:10 2024 +1000

    KVM: PPC: Book3S HV: Prevent UAF in kvm_spapr_tce_attach_iommu_group()
    
    [ Upstream commit a986fa57fd81a1430e00b3c6cf8a325d6f894a63 ]
    
    Al reported a possible use-after-free (UAF) in kvm_spapr_tce_attach_iommu_group().
    
    It looks up `stt` from tablefd, but then continues to use it after doing
    fdput() on the returned fd. After the fdput() the tablefd is free to be
    closed by another thread. The close calls kvm_spapr_tce_release() and
    then release_spapr_tce_table() (via call_rcu()) which frees `stt`.
    
    Although there are calls to rcu_read_lock() in
    kvm_spapr_tce_attach_iommu_group() they are not sufficient to prevent
    the UAF, because `stt` is used outside the locked regions.
    
    With an artifcial delay after the fdput() and a userspace program which
    triggers the race, KASAN detects the UAF:
    
      BUG: KASAN: slab-use-after-free in kvm_spapr_tce_attach_iommu_group+0x298/0x720 [kvm]
      Read of size 4 at addr c000200027552c30 by task kvm-vfio/2505
      CPU: 54 PID: 2505 Comm: kvm-vfio Not tainted 6.10.0-rc3-next-20240612-dirty #1
      Hardware name: 8335-GTH POWER9 0x4e1202 opal:skiboot-v6.5.3-35-g1851b2a06 PowerNV
      Call Trace:
        dump_stack_lvl+0xb4/0x108 (unreliable)
        print_report+0x2b4/0x6ec
        kasan_report+0x118/0x2b0
        __asan_load4+0xb8/0xd0
        kvm_spapr_tce_attach_iommu_group+0x298/0x720 [kvm]
        kvm_vfio_set_attr+0x524/0xac0 [kvm]
        kvm_device_ioctl+0x144/0x240 [kvm]
        sys_ioctl+0x62c/0x1810
        system_call_exception+0x190/0x440
        system_call_vectored_common+0x15c/0x2ec
      ...
      Freed by task 0:
       ...
       kfree+0xec/0x3e0
       release_spapr_tce_table+0xd4/0x11c [kvm]
       rcu_core+0x568/0x16a0
       handle_softirqs+0x23c/0x920
       do_softirq_own_stack+0x6c/0x90
       do_softirq_own_stack+0x58/0x90
       __irq_exit_rcu+0x218/0x2d0
       irq_exit+0x30/0x80
       arch_local_irq_restore+0x128/0x230
       arch_local_irq_enable+0x1c/0x30
       cpuidle_enter_state+0x134/0x5cc
       cpuidle_enter+0x6c/0xb0
       call_cpuidle+0x7c/0x100
       do_idle+0x394/0x410
       cpu_startup_entry+0x60/0x70
       start_secondary+0x3fc/0x410
       start_secondary_prolog+0x10/0x14
    
    Fix it by delaying the fdput() until `stt` is no longer in use, which
    is effectively the entire function. To keep the patch minimal add a call
    to fdput() at each of the existing return paths. Future work can convert
    the function to goto or __cleanup style cleanup.
    
    With the fix in place the test case no longer triggers the UAF.
    
    Reported-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
    Closes: https://lore.kernel.org/all/20240610024437.GA1464458@ZenIV/
    Signed-off-by: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
    Link: https://msgid.link/20240614122910.3499489-1-mpe@xxxxxxxxxxxxxx
    Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>

diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index b569ebaa590e2..3ff3de9a52acf 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -130,14 +130,16 @@ long kvm_spapr_tce_attach_iommu_group(struct kvm *kvm, int tablefd,
 	}
 	rcu_read_unlock();
 
-	fdput(f);
-
-	if (!found)
+	if (!found) {
+		fdput(f);
 		return -EINVAL;
+	}
 
 	table_group = iommu_group_get_iommudata(grp);
-	if (WARN_ON(!table_group))
+	if (WARN_ON(!table_group)) {
+		fdput(f);
 		return -EFAULT;
+	}
 
 	for (i = 0; i < IOMMU_TABLE_GROUP_MAX_TABLES; ++i) {
 		struct iommu_table *tbltmp = table_group->tables[i];
@@ -158,8 +160,10 @@ long kvm_spapr_tce_attach_iommu_group(struct kvm *kvm, int tablefd,
 			break;
 		}
 	}
-	if (!tbl)
+	if (!tbl) {
+		fdput(f);
 		return -EINVAL;
+	}
 
 	rcu_read_lock();
 	list_for_each_entry_rcu(stit, &stt->iommu_tables, next) {
@@ -170,6 +174,7 @@ long kvm_spapr_tce_attach_iommu_group(struct kvm *kvm, int tablefd,
 			/* stit is being destroyed */
 			iommu_tce_table_put(tbl);
 			rcu_read_unlock();
+			fdput(f);
 			return -ENOTTY;
 		}
 		/*
@@ -177,6 +182,7 @@ long kvm_spapr_tce_attach_iommu_group(struct kvm *kvm, int tablefd,
 		 * its KVM reference counter and can return.
 		 */
 		rcu_read_unlock();
+		fdput(f);
 		return 0;
 	}
 	rcu_read_unlock();
@@ -184,6 +190,7 @@ long kvm_spapr_tce_attach_iommu_group(struct kvm *kvm, int tablefd,
 	stit = kzalloc(sizeof(*stit), GFP_KERNEL);
 	if (!stit) {
 		iommu_tce_table_put(tbl);
+		fdput(f);
 		return -ENOMEM;
 	}
 
@@ -192,6 +199,7 @@ long kvm_spapr_tce_attach_iommu_group(struct kvm *kvm, int tablefd,
 
 	list_add_rcu(&stit->next, &stt->iommu_tables);
 
+	fdput(f);
 	return 0;
 }
 




[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux