ANDROID: KVM: arm64: Rework pKVM module locking

Much of the functions related to pKVM modules at EL2 are currently
guarded by the pkvm_modules_lock. This lock is only useful in case of
module hypercalls racing with __pkvm_close_module_registration().
However, the latter is by definition a privileged operation, so we can
trust that the host is not trying to exploit races to attack EL2 at this
point.

As such, let's remove the pkvm_modules_lock and re-use the existing
concept of privileged hypercalls instead. To do so, the hypercall that
limits the privileged range is moved dynamically depending on whether
pKVM modules are supported or not.

Bug: 264070847
Change-Id: I6924471339f2123ab244cdb71ffcb2a299fa75a4
Signed-off-by: Quentin Perret <qperret@google.com>

arch/arm64/include/asm/kvm_asm.h

@@ -63,6 +63,17 @@ enum __kvm_host_smccc_func {
 	__KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid_ipa,
 	__KVM_HOST_SMCCC_FUNC___kvm_tlb_flush_vmid,
 	__KVM_HOST_SMCCC_FUNC___kvm_flush_cpu_context,
+
+	/*
+	 * __pkvm_alloc_module_va may temporarily serve as the privileged hcall
+	 * limit when module loading is enabled, see early_pkvm_enable_modules().
+	 */
+	__KVM_HOST_SMCCC_FUNC___pkvm_alloc_module_va,
+	__KVM_HOST_SMCCC_FUNC___pkvm_map_module_page,
+	__KVM_HOST_SMCCC_FUNC___pkvm_unmap_module_page,
+	__KVM_HOST_SMCCC_FUNC___pkvm_init_module,
+	__KVM_HOST_SMCCC_FUNC___pkvm_register_hcall,
+	__KVM_HOST_SMCCC_FUNC___pkvm_close_module_registration,
 	__KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize,
 
 	/* Hypercalls available after pKVM finalisation */
@@ -85,12 +96,6 @@ enum __kvm_host_smccc_func {
 	__KVM_HOST_SMCCC_FUNC___pkvm_iommu_register,
 	__KVM_HOST_SMCCC_FUNC___pkvm_iommu_pm_notify,
 	__KVM_HOST_SMCCC_FUNC___pkvm_iommu_finalize,
-	__KVM_HOST_SMCCC_FUNC___pkvm_register_hcall,
-	__KVM_HOST_SMCCC_FUNC___pkvm_alloc_module_va,
-	__KVM_HOST_SMCCC_FUNC___pkvm_map_module_page,
-	__KVM_HOST_SMCCC_FUNC___pkvm_unmap_module_page,
-	__KVM_HOST_SMCCC_FUNC___pkvm_init_module,
-	__KVM_HOST_SMCCC_FUNC___pkvm_close_module_registration,
 
 	/*
 	 * Start of the dynamically registered hypercalls. Start a bit

arch/arm64/kvm/hyp/include/nvhe/modules.h

@@ -11,6 +11,8 @@ int __pkvm_register_hyp_panic_notifier(void (*cb)(struct kvm_cpu_context *));
 enum pkvm_psci_notification;
 int __pkvm_register_psci_notifier(void (*cb)(enum pkvm_psci_notification, struct kvm_cpu_context *));
 
+int reset_pkvm_priv_hcall_limit(void);
+
 #ifdef CONFIG_MODULES
 int __pkvm_init_module(void *module_init);
 int __pkvm_register_hcall(unsigned long hfn_hyp_va);

arch/arm64/kvm/hyp/nvhe/hyp-main.c

@@ -1225,6 +1225,13 @@ static const hcall_t host_hcall[] = {
 	HANDLE_FUNC(__kvm_tlb_flush_vmid_ipa),
 	HANDLE_FUNC(__kvm_tlb_flush_vmid),
 	HANDLE_FUNC(__kvm_flush_cpu_context),
+
+	HANDLE_FUNC(__pkvm_alloc_module_va),
+	HANDLE_FUNC(__pkvm_map_module_page),
+	HANDLE_FUNC(__pkvm_unmap_module_page),
+	HANDLE_FUNC(__pkvm_init_module),
+	HANDLE_FUNC(__pkvm_register_hcall),
+	HANDLE_FUNC(__pkvm_close_module_registration),
 	HANDLE_FUNC(__pkvm_prot_finalize),
 
 	HANDLE_FUNC(__pkvm_host_share_hyp),
@@ -1246,14 +1253,24 @@ static const hcall_t host_hcall[] = {
 	HANDLE_FUNC(__pkvm_iommu_register),
 	HANDLE_FUNC(__pkvm_iommu_pm_notify),
 	HANDLE_FUNC(__pkvm_iommu_finalize),
-	HANDLE_FUNC(__pkvm_alloc_module_va),
-	HANDLE_FUNC(__pkvm_map_module_page),
-	HANDLE_FUNC(__pkvm_unmap_module_page),
-	HANDLE_FUNC(__pkvm_init_module),
-	HANDLE_FUNC(__pkvm_register_hcall),
-	HANDLE_FUNC(__pkvm_close_module_registration),
 };
 
+unsigned long pkvm_priv_hcall_limit __ro_after_init = __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize;
+
+int reset_pkvm_priv_hcall_limit(void)
+{
+	unsigned long *addr;
+
+	if (pkvm_priv_hcall_limit == __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize)
+		return -EACCES;
+
+	addr = hyp_fixmap_map(__hyp_pa(&pkvm_priv_hcall_limit));
+	*addr = KVM_HOST_SMCCC_FUNC(__pkvm_prot_finalize);
+	hyp_fixmap_unmap();
+
+	return 0;
+}
+
 static void handle_host_hcall(struct kvm_cpu_context *host_ctxt)
 {
 	DECLARE_REG(unsigned long, id, host_ctxt, 0);
@@ -1273,7 +1290,7 @@ static void handle_host_hcall(struct kvm_cpu_context *host_ctxt)
 	 * returns -EPERM after the first call for a given CPU.
 	 */
 	if (static_branch_unlikely(&kvm_protected_mode_initialized))
-		hcall_min = __KVM_HOST_SMCCC_FUNC___pkvm_prot_finalize;
+		hcall_min = pkvm_priv_hcall_limit;
 
 	id -= KVM_HOST_SMCCC_ID(0);
 

arch/arm64/kvm/hyp/nvhe/mm.c

@@ -105,11 +105,14 @@ int __pkvm_create_private_mapping(phys_addr_t phys, size_t size,
 #ifdef CONFIG_NVHE_EL2_DEBUG
 static unsigned long mod_range_start = ULONG_MAX;
 static unsigned long mod_range_end;
+static DEFINE_HYP_SPINLOCK(mod_range_lock);
 
 static void update_mod_range(unsigned long addr, size_t size)
 {
+	hyp_spin_lock(&mod_range_lock);
 	mod_range_start = min(mod_range_start, addr);
 	mod_range_end = max(mod_range_end, addr + size);
+	hyp_spin_unlock(&mod_range_lock);
 }
 
 static void assert_in_mod_range(unsigned long addr)
@@ -119,7 +122,9 @@ static void assert_in_mod_range(unsigned long addr)
 	 * allocations between modules being loaded, but in practice that is
 	 * probably going to be allocation initiated by the modules themselves.
 	 */
+	hyp_spin_lock(&mod_range_lock);
 	WARN_ON(addr < mod_range_start || mod_range_end <= addr);
+	hyp_spin_unlock(&mod_range_lock);
 }
 #else
 static inline void update_mod_range(unsigned long addr, size_t size) { }
@@ -131,12 +136,8 @@ void *__pkvm_alloc_module_va(u64 nr_pages)
 	size_t size = nr_pages << PAGE_SHIFT;
 	unsigned long addr = 0;
 
-	pkvm_modules_lock();
-	if (pkvm_modules_enabled()) {
-		if (!pkvm_alloc_private_va_range(size, &addr))
-			update_mod_range(addr, size);
-	}
-	pkvm_modules_unlock();
+	if (!pkvm_alloc_private_va_range(size, &addr))
+		update_mod_range(addr, size);
 
 	return (void *)addr;
 }
@@ -144,34 +145,27 @@ void *__pkvm_alloc_module_va(u64 nr_pages)
 int __pkvm_map_module_page(u64 pfn, void *va, enum kvm_pgtable_prot prot)
 {
 	unsigned long addr = (unsigned long)va;
-	int ret = -EACCES;
-
-	pkvm_modules_lock();
+	int ret;
 
 	assert_in_mod_range(addr);
 
-	if (!pkvm_modules_enabled())
-		goto err;
-
 	ret = __pkvm_host_donate_hyp(pfn, 1);
 	if (ret)
-		goto err;
+		return ret;
 
 	ret = __pkvm_create_mappings(addr, PAGE_SIZE, hyp_pfn_to_phys(pfn), prot);
-err:
-	pkvm_modules_unlock();
+	if (ret) {
+		WARN_ON(__pkvm_hyp_donate_host(pfn, 1));
+		return ret;
+	}
 
-	return ret;
+	return 0;
 }
 
 void __pkvm_unmap_module_page(u64 pfn, void *va)
 {
-	pkvm_modules_lock();
-	if (pkvm_modules_enabled()) {
-		WARN_ON(__pkvm_hyp_donate_host(pfn, 1));
-		pkvm_remove_mappings(va, va + PAGE_SIZE);
-	}
-	pkvm_modules_unlock();
+	WARN_ON(__pkvm_hyp_donate_host(pfn, 1));
+	pkvm_remove_mappings(va, va + PAGE_SIZE);
 }
 
 int pkvm_create_mappings_locked(void *from, void *to, enum kvm_pgtable_prot prot)

arch/arm64/kvm/hyp/nvhe/modules.c

@@ -17,26 +17,7 @@ static void __kvm_flush_dcache_to_poc(void *addr, size_t size)
 	kvm_flush_dcache_to_poc((unsigned long)addr, (unsigned long)size);
 }
 
-DEFINE_HYP_SPINLOCK(modules_lock);
-
-bool __pkvm_modules_enabled __ro_after_init;
-
-void pkvm_modules_lock(void)
-{
-	hyp_spin_lock(&modules_lock);
-}
-
-void pkvm_modules_unlock(void)
-{
-	hyp_spin_unlock(&modules_lock);
-}
-
-bool pkvm_modules_enabled(void)
-{
-	return __pkvm_modules_enabled;
-}
-
-static u64 early_lm_pages;
+static atomic_t early_lm_pages;
 static void *__pkvm_linear_map_early(phys_addr_t phys, size_t size, enum kvm_pgtable_prot prot)
 {
 	void *addr = NULL;
@@ -45,52 +26,33 @@ static void *__pkvm_linear_map_early(phys_addr_t phys, size_t size, enum kvm_pgt
 	if (!PAGE_ALIGNED(phys) || !PAGE_ALIGNED(size))
 		return NULL;
 
-	pkvm_modules_lock();
-	if (!__pkvm_modules_enabled)
-		goto out;
-
 	addr = __hyp_va(phys);
 	ret = pkvm_create_mappings(addr, addr + size, prot);
 	if (ret)
 		addr = NULL;
 	else
-		early_lm_pages += size >> PAGE_SHIFT;
-out:
-	pkvm_modules_unlock();
+		atomic_add(size, &early_lm_pages);
 
 	return addr;
 }
 
 static void __pkvm_linear_unmap_early(void *addr, size_t size)
 {
-	pkvm_modules_lock();
 	pkvm_remove_mappings(addr, addr + size);
-	early_lm_pages -= size >> PAGE_SHIFT;
-	pkvm_modules_unlock();
+	atomic_sub(size, &early_lm_pages);
 }
 
 int __pkvm_close_module_registration(void)
 {
-	int ret;
-
-	pkvm_modules_lock();
 	/*
 	 * Page ownership tracking might go out of sync if there are stale
 	 * entries in pKVM's linear map range, so they must really be gone by
 	 * now.
 	 */
-	WARN_ON(early_lm_pages);
-
-	ret = __pkvm_modules_enabled ? 0 : -EACCES;
-	if (!ret) {
-		void *addr = hyp_fixmap_map(__hyp_pa(&__pkvm_modules_enabled));
-		*(bool *)addr = false;
-		hyp_fixmap_unmap();
-	}
-	pkvm_modules_unlock();
+	WARN_ON(atomic_read(&early_lm_pages));
+	return reset_pkvm_priv_hcall_limit();
 
 	/* The fuse is blown! No way back until reset */
-	return ret;
 }
 
 const struct pkvm_module_ops module_ops = {
@@ -123,18 +85,8 @@ const struct pkvm_module_ops module_ops = {
 int __pkvm_init_module(void *module_init)
 {
 	int (*do_module_init)(const struct pkvm_module_ops *ops) = module_init;
-	int ret;
 
-	pkvm_modules_lock();
-	if (!pkvm_modules_enabled()) {
-		ret = -EACCES;
-		goto err;
-	}
-	ret = do_module_init(&module_ops);
-err:
-	pkvm_modules_unlock();
-
-	return ret;
+	return do_module_init(&module_ops);
 }
 
 #define MAX_DYNAMIC_HCALLS 128
@@ -183,12 +135,6 @@ int __pkvm_register_hcall(unsigned long hvn_hyp_va)
 	dyn_hcall_t hfn = (void *)hvn_hyp_va;
 	int reserved_id, ret;
 
-	pkvm_modules_lock();
-	if (!pkvm_modules_enabled()) {
-		ret = -EACCES;
-		goto err;
-	}
-
 	hyp_spin_lock(&dyn_hcall_lock);
 
 	reserved_id = atomic_read(&num_dynamic_hcalls);
@@ -209,8 +155,6 @@ int __pkvm_register_hcall(unsigned long hvn_hyp_va)
 	ret = reserved_id + __KVM_HOST_SMCCC_FUNC___dynamic_hcalls;
 err_hcall_unlock:
 	hyp_spin_unlock(&dyn_hcall_lock);
-err:
-	pkvm_modules_unlock();
 
 	return ret;
 };

arch/arm64/kvm/pkvm.c

@@ -502,7 +502,13 @@ int pkvm_vm_ioctl_enable_cap(struct kvm *kvm, struct kvm_enable_cap *cap)
 #ifdef CONFIG_MODULES
 static int __init early_pkvm_enable_modules(char *arg)
 {
-	kvm_nvhe_sym(__pkvm_modules_enabled) = true;
+	extern unsigned long kvm_nvhe_sym(pkvm_priv_hcall_limit);
+
+	/*
+	 * Move the limit to allow module loading HVCs. It will be moved back to
+	 * its original position in __pkvm_close_module_registration().
+	 */
+	kvm_nvhe_sym(pkvm_priv_hcall_limit) = __KVM_HOST_SMCCC_FUNC___pkvm_alloc_module_va;
 
 	return 0;
 }