[PATCH] RISC-V: KVM: Fix heap overflow in SBI PMU event info handler

[PATCH] RISC-V: KVM: Fix heap overflow in SBI PMU event info handler

[Naveed Khan]

kvm_riscv_vcpu_pmu_event_info() implements the SBI PMU EVENT_GET_INFO
call. It sizes the temporary shared-memory buffer that mirrors the
guest's event-info array with:

	int shmem_size = num_events * sizeof(*einfo);

num_events comes directly from the guest (SBI argument a2) and is only
rejected when it is zero; there is no upper bound. sizeof(*einfo) is 16,
so the multiplication is evaluated as size_t but the result is then
truncated when it is stored into the 32-bit int shmem_size.

A guest can pass, for example, num_events = 0x10000001. The real size is
0x100000010 bytes, but shmem_size truncates to 16. kzalloc() then
allocates room for a single entry while the following loop still runs the
full 64-bit num_events times:

	for (int i = 0; i < num_events; i++) {
		...
		einfo[i].output = (ret > 0) ? 1 : 0;
	}

This is a guest-triggerable heap out-of-bounds read and write that
extends far past the allocation.

Compute the size with size_mul() into a size_t so the value used for the
allocation always matches the loop bound. For an oversized num_events the
multiplication saturates to SIZE_MAX, kzalloc() fails and the call
returns SBI_ERR_FAILURE before the loop runs; well-formed requests behave
exactly as before.

Signed-off-by: Naveed Khan <naveed@digiscrypt.com>
---
diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c
index bb46dcbfb2..ff940fa74b 100644
--- a/arch/riscv/kvm/vcpu_pmu.c
+++ b/arch/riscv/kvm/vcpu_pmu.c
@@ -12,6 +12,7 @@
 #include <linux/err.h>
 #include <linux/kvm_host.h>
 #include <linux/nospec.h>
+#include <linux/overflow.h>
 #include <linux/perf/riscv_pmu.h>
 #include <asm/csr.h>
 #include <asm/kvm_isa.h>
@@ -479,7 +480,7 @@ int kvm_riscv_vcpu_pmu_event_info(struct kvm_vcpu *vcpu, unsigned long saddr_low
 				  unsigned long flags, struct kvm_vcpu_sbi_return *retdata)
 {
 	struct riscv_pmu_event_info *einfo = NULL;
-	int shmem_size = num_events * sizeof(*einfo);
+	size_t shmem_size = size_mul(num_events, sizeof(*einfo));
 	gpa_t shmem;
 	u32 eidx, etype;
 	u64 econfig;
-- 
2.50.1 (Apple Git-155)

Re: [PATCH] RISC-V: KVM: Fix heap overflow in SBI PMU event info handler

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] An unbounded, guest-controlled `num_events` parameter translates into a large memory allocation, triggering a `WARN_ON_ONCE` in the host's page allocator.
--

--- Patch [1]: [PATCH] RISC-V: KVM: Fix heap overflow in SBI PMU event info handler ---
commit 59f34c484dfc616073369a4a2f4ea4eb32bc7ab5
Author: Naveed Khan <naveed@digiscrypt.com>

RISC-V: KVM: Fix heap overflow in SBI PMU event info handler

This commit fixes a heap out-of-bounds read and write by using size_mul()
to calculate the shared memory size, saturating to SIZE_MAX on overflow
so the allocation fails safely.

> diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c
> index bb46dcbfb24da..ff940fa74b55d 100644
> --- a/arch/riscv/kvm/vcpu_pmu.c
> +++ b/arch/riscv/kvm/vcpu_pmu.c

[ ... ]

> @@ -479,7 +480,7 @@ int kvm_riscv_vcpu_pmu_event_info(struct kvm_vcpu *vcpu, unsigned long saddr_low
>  				  unsigned long flags, struct kvm_vcpu_sbi_return *retdata)
>  {
>  	struct riscv_pmu_event_info *einfo = NULL;
> -	int shmem_size = num_events * sizeof(*einfo);
> +	size_t shmem_size = size_mul(num_events, sizeof(*einfo));

[Severity: High]
This is a pre-existing issue, but does this allocation still allow a guest
to trigger a host warning?

Because num_events comes directly from the guest without an upper bound, a
malicious guest can provide an extremely large value. While size_mul() now
safely saturates to SIZE_MAX and prevents the loop mismatch, this massive
size is subsequently passed to kzalloc():

arch/riscv/kvm/vcpu_pmu.c:kvm_riscv_vcpu_pmu_event_info() {
    ...
    einfo = kzalloc(shmem_size, GFP_KERNEL);
    ...
}

Since GFP_KERNEL lacks the __GFP_NOWARN flag, passing a size that evaluates
to an order larger than MAX_PAGE_ORDER will trigger a WARN_ON_ONCE in the
host's page allocator.

If panic_on_warn is enabled on the host, this results in a guest-triggerable
host denial of service. Should this allocation include __GFP_NOWARN, or
should num_events be explicitly bounded before the allocation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/178345245327.72065.13249716450708539854@digiscrypt.com?part=1