From b366878684444ff38a25fb9c1bd55d823b2f22f1 Mon Sep 17 00:00:00 2001 From: Quentin Perret Date: Fri, 7 Oct 2022 12:27:51 +0000 Subject: [PATCH 1/6] ANDROID: KVM: arm64: Fix MMIO guard unmap MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pierre-Clément reports that the MMIO guard unmap hypercall exposed to protected guests returns an error upon success. Indeed, SMCCC_RET_SUCCESS is returned only if __pkvm_remove_ioguard_page() failed, which doesn't match the expected behaviour. Fix this by returning SMCCC_RET_INVALID_PARAMETER instead. Bug: 251426790 Reported-by: Pierre-Clément Tosi Signed-off-by: Quentin Perret Change-Id: Id746fa7d5d3a03ee5df6d114a07240822a0be93b --- arch/arm64/kvm/hyp/nvhe/pkvm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c index 50717a46e735..3caa670a18cb 100644 --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c @@ -1338,6 +1338,8 @@ bool kvm_handle_pvm_hvc64(struct kvm_vcpu *vcpu, u64 *exit_code) return pkvm_install_ioguard_page(vcpu, exit_code); case ARM_SMCCC_VENDOR_HYP_KVM_MMIO_GUARD_UNMAP_FUNC_ID: if (__pkvm_remove_ioguard_page(vcpu, vcpu_get_reg(vcpu, 1))) + val[0] = SMCCC_RET_INVALID_PARAMETER; + else val[0] = SMCCC_RET_SUCCESS; break; case ARM_SMCCC_VENDOR_HYP_KVM_MMIO_GUARD_INFO_FUNC_ID: From 07e046b9966621bc061fcc23e81c76bdb8cc56b1 Mon Sep 17 00:00:00 2001 From: Lee Jones Date: Fri, 8 Jul 2022 08:40:09 +0100 Subject: [PATCH 2/6] BACKPORT: HID: steam: Prevent NULL pointer dereference in steam_{recv,send}_report commit cd11d1a6114bd4bc6450ae59f6e110ec47362126 upstream. It is possible for a malicious device to forgo submitting a Feature Report. The HID Steam driver presently makes no prevision for this and de-references the 'struct hid_report' pointer obtained from the HID devices without first checking its validity. Let's change that. Bug: 223455965 Cc: Jiri Kosina Cc: Benjamin Tissoires Cc: linux-input@vger.kernel.org Fixes: c164d6abf3841 ("HID: add driver for Valve Steam Controller") Signed-off-by: Lee Jones Signed-off-by: Jiri Kosina Signed-off-by: Greg Kroah-Hartman Signed-off-by: Lee Jones Change-Id: Ica12507b87309a7c46b4cab6fcfe4499cd96f45d --- drivers/hid/hid-steam.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/hid/hid-steam.c b/drivers/hid/hid-steam.c index a3b151b29bd7..fc616db4231b 100644 --- a/drivers/hid/hid-steam.c +++ b/drivers/hid/hid-steam.c @@ -134,6 +134,11 @@ static int steam_recv_report(struct steam_device *steam, int ret; r = steam->hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[0]; + if (!r) { + hid_err(steam->hdev, "No HID_FEATURE_REPORT submitted - nothing to read\n"); + return -EINVAL; + } + if (hid_report_len(r) < 64) return -EINVAL; @@ -165,6 +170,11 @@ static int steam_send_report(struct steam_device *steam, int ret; r = steam->hdev->report_enum[HID_FEATURE_REPORT].report_id_hash[0]; + if (!r) { + hid_err(steam->hdev, "No HID_FEATURE_REPORT submitted - nothing to read\n"); + return -EINVAL; + } + if (hid_report_len(r) < 64) return -EINVAL; From e4c738dd1383f550387bc85e503d35b6dfc15274 Mon Sep 17 00:00:00 2001 From: Pat Tjin Date: Tue, 11 Oct 2022 14:02:15 -0700 Subject: [PATCH 3/6] ANDROID: Update the ABI representation 4 function symbol(s) added 'int __hid_register_driver(struct hid_driver *, struct module *, const char *)' 'int hid_hw_start(struct hid_device *, unsigned int)' 'int hid_open_report(struct hid_device *)' 'void hid_unregister_driver(struct hid_driver *)' Bug: 246795245 Signed-off-by: Pat Tjin Change-Id: Ic9a4187a7215e5678b8196fad4d4350802d77a10 --- android/abi_gki_aarch64.xml | 46 ++++++++++++++++++++++++++++++++- android/abi_gki_aarch64_generic | 4 +++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/android/abi_gki_aarch64.xml b/android/abi_gki_aarch64.xml index 532be833ec36..06d41dd2254d 100644 --- a/android/abi_gki_aarch64.xml +++ b/android/abi_gki_aarch64.xml @@ -91,6 +91,7 @@ + @@ -1712,8 +1713,11 @@ + + + @@ -9975,7 +9979,23 @@ - + + + + + + + + + + + + + + + + + @@ -45164,6 +45184,11 @@ + + + + + @@ -109954,6 +109979,12 @@ + + + + + + @@ -118893,6 +118924,11 @@ + + + + + @@ -118901,12 +118937,20 @@ + + + + + + + + diff --git a/android/abi_gki_aarch64_generic b/android/abi_gki_aarch64_generic index 81746e107e32..d9a45368005a 100644 --- a/android/abi_gki_aarch64_generic +++ b/android/abi_gki_aarch64_generic @@ -993,8 +993,12 @@ hid_allocate_device hid_debug hid_destroy_device + hid_hw_start hid_input_report + hid_open_report hid_parse_report + __hid_register_driver + hid_unregister_driver hrtimer_active hrtimer_cancel hrtimer_forward From 32bef95f9124f76f8b6117702a15b1206781e077 Mon Sep 17 00:00:00 2001 From: Johannes Weiner Date: Mon, 3 May 2021 13:49:17 -0400 Subject: [PATCH 4/6] UPSTREAM: psi: Fix psi state corruption when schedule() races with cgroup move 4117cebf1a9f ("psi: Optimize task switch inside shared cgroups") introduced a race condition that corrupts internal psi state. This manifests as kernel warnings, sometimes followed by bogusly high IO pressure: psi: task underflow! cpu=1 t=2 tasks=[0 0 0 0] clear=c set=0 (schedule() decreasing RUNNING and ONCPU, both of which are 0) psi: incosistent task state! task=2412744:systemd cpu=17 psi_flags=e clear=3 set=0 (cgroup_move_task() clearing MEMSTALL and IOWAIT, but task is MEMSTALL | RUNNING | ONCPU) What the offending commit does is batch the two psi callbacks in schedule() to reduce the number of cgroup tree updates. When prev is deactivated and removed from the runqueue, nothing is done in psi at first; when the task switch completes, TSK_RUNNING and TSK_IOWAIT are updated along with TSK_ONCPU. However, the deactivation and the task switch inside schedule() aren't atomic: pick_next_task() may drop the rq lock for load balancing. When this happens, cgroup_move_task() can run after the task has been physically dequeued, but the psi updates are still pending. Since it looks at the task's scheduler state, it doesn't move everything to the new cgroup that the task switch that follows is about to clear from it. cgroup_move_task() will leak the TSK_RUNNING count in the old cgroup, and psi_sched_switch() will underflow it in the new cgroup. A similar thing can happen for iowait. TSK_IOWAIT is usually set when a p->in_iowait task is dequeued, but again this update is deferred to the switch. cgroup_move_task() can see an unqueued p->in_iowait task and move a non-existent TSK_IOWAIT. This results in the inconsistent task state warning, as well as a counter underflow that will result in permanent IO ghost pressure being reported. Fix this bug by making cgroup_move_task() use task->psi_flags instead of looking at the potentially mismatching scheduler state. [ We used the scheduler state historically in order to not rely on task->psi_flags for anything but debugging. But that ship has sailed anyway, and this is simpler and more robust. We previously already batched TSK_ONCPU clearing with the TSK_RUNNING update inside the deactivation call from schedule(). But that ordering was safe and didn't result in TSK_ONCPU corruption: unlike most places in the scheduler, cgroup_move_task() only checked task_current() and handled TSK_ONCPU if the task was still queued. ] bug: b/253347377 Fixes: 4117cebf1a9f ("psi: Optimize task switch inside shared cgroups") Signed-off-by: Johannes Weiner Signed-off-by: Peter Zijlstra (Intel) Link: https://lkml.kernel.org/r/20210503174917.38579-1-hannes@cmpxchg.org (cherry picked from commit d583d360a620e6229422b3455d0be082b8255f5e) Change-Id: Id0a292058d4bffb716d8e1496f72139e8d435410 --- kernel/sched/psi.c | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/kernel/sched/psi.c b/kernel/sched/psi.c index f46ac0f39777..e14917cd2a1d 100644 --- a/kernel/sched/psi.c +++ b/kernel/sched/psi.c @@ -1020,7 +1020,7 @@ void psi_cgroup_free(struct cgroup *cgroup) */ void cgroup_move_task(struct task_struct *task, struct css_set *to) { - unsigned int task_flags = 0; + unsigned int task_flags; struct rq_flags rf; struct rq *rq; @@ -1035,15 +1035,31 @@ void cgroup_move_task(struct task_struct *task, struct css_set *to) rq = task_rq_lock(task, &rf); - if (task_on_rq_queued(task)) { - task_flags = TSK_RUNNING; - if (task_current(rq, task)) - task_flags |= TSK_ONCPU; - } else if (task->in_iowait) - task_flags = TSK_IOWAIT; - - if (task->in_memstall) - task_flags |= TSK_MEMSTALL; + /* + * We may race with schedule() dropping the rq lock between + * deactivating prev and switching to next. Because the psi + * updates from the deactivation are deferred to the switch + * callback to save cgroup tree updates, the task's scheduling + * state here is not coherent with its psi state: + * + * schedule() cgroup_move_task() + * rq_lock() + * deactivate_task() + * p->on_rq = 0 + * psi_dequeue() // defers TSK_RUNNING & TSK_IOWAIT updates + * pick_next_task() + * rq_unlock() + * rq_lock() + * psi_task_change() // old cgroup + * task->cgroups = to + * psi_task_change() // new cgroup + * rq_unlock() + * rq_lock() + * psi_sched_switch() // does deferred updates in new cgroup + * + * Don't rely on the scheduling state. Use psi_flags instead. + */ + task_flags = task->psi_flags; if (task_flags) psi_task_change(task, task_flags, 0); From f177a280b1e5f99759fb029f4470131ea8a2c375 Mon Sep 17 00:00:00 2001 From: Quentin Perret Date: Fri, 14 Oct 2022 13:42:37 +0000 Subject: [PATCH 5/6] ANDROID: KVM: arm64: Fix MMIO guard map error handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pierre-Clément reports that the error codes returned by the MMIO guard map hypercall may end up being incorrectly reported as positive to callers who interpret them a signed 64-bit integers, as specified in the SMCCC. Fix this by storing the return value in a 64-bit variable instead. Bug: 253586500 Reported-by: Pierre-Clément Tosi Signed-off-by: Quentin Perret Change-Id: I3092856ec1a1fd1648a75c9e4ad4bfebd8830d14 --- arch/arm64/kvm/hyp/nvhe/pkvm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c index 3caa670a18cb..b9e6337852dd 100644 --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c @@ -1246,7 +1246,7 @@ out_guest_err: static bool pkvm_install_ioguard_page(struct kvm_vcpu *vcpu, u64 *exit_code) { - u32 retval = SMCCC_RET_SUCCESS; + u64 retval = SMCCC_RET_SUCCESS; u64 ipa = smccc_get_arg1(vcpu); int ret; From 19424168db55715a7ba3f00505ac9b9f19fd3705 Mon Sep 17 00:00:00 2001 From: Quentin Perret Date: Fri, 14 Oct 2022 14:25:43 +0000 Subject: [PATCH 6/6] ANDROID: KVM: arm64: Force CMOs with FWB when reclaiming guest pages __clean_dcache_guest_page() is optimized to elide cache maintenance operations on CPUs with FWB. The underlying assumption is that FWB is always used by KVM when available. Although correct in the normal KVM world, pKVM actively disables FWB for the host stage-2. As such, omitting CMOs when guest memory is being reclaimed may provide a malicious host with the ability to read the content of the recently reclaimed pages. Fix this by using the lower level kvm_flush_dcache_to_poc() helper directly from the reclaim path. Bug: 243501419 Reported-by: Will Deacon Signed-off-by: Quentin Perret Change-Id: I8e96ef7a8ccab2a59d3df46cd4d1a73190a2f457 --- arch/arm64/kvm/hyp/nvhe/mem_protect.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c index 6ca172ac3445..810436d85608 100644 --- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c +++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c @@ -1915,7 +1915,14 @@ static int hyp_zero_page(phys_addr_t phys) if (!addr) return -EINVAL; memset(addr, 0, PAGE_SIZE); - __clean_dcache_guest_page(addr, PAGE_SIZE); + /* + * Prefer kvm_flush_dcache_to_poc() over __clean_dcache_guest_page() + * here as the latter may elide the CMO under the assumption that FWB + * will be enabled on CPUs that support it. This is incorrect for the + * host stage-2 and would otherwise lead to a malicious host potentially + * being able to read the content of newly reclaimed guest pages. + */ + kvm_flush_dcache_to_poc(addr, PAGE_SIZE); return hyp_fixmap_unmap(); }