Re: [PATCH v2] RISC-V: KVM: Fix PMU event info array size overflow

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unbounded guest-controlled `num_events` allows arbitrarily large memory allocations via `kvcalloc` and host soft lockups.
--

--- Patch [1]: [PATCH v2] RISC-V: KVM: Fix PMU event info array size overflow ---
commit 500aa03e09c05223e23f66e67acfa12d3e264362
Author: Guidong Han <2045gemini@gmail.com>

RISC-V: KVM: Fix PMU event info array size overflow

This commit fixes a slab out-of-bounds issue by storing the shared memory
size in a size_t and checking for multiplication overflow. It also switches
the allocation to use kvcalloc() with GFP_KERNEL_ACCOUNT and __GFP_NOWARN
to support large guest requests safely.

Link: https://lore.kernel.org/kvm/178345245327.72065.13249716450708539854@digiscrypt.com/
Link: https://lore.kernel.org/kvm/20260718153620.829378-1-2045gemini@gmail.com/

> diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c
> index 2025b664961c8..a7f948410d531 100644
> --- a/arch/riscv/kvm/vcpu_pmu.c
> +++ b/arch/riscv/kvm/vcpu_pmu.c
[ ... ]
> @@ -479,13 +481,14 @@ 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;
>  	gpa_t shmem;
[ ... ]
> -	if (flags != 0 || (saddr_low & (SZ_16 - 1) || num_events == 0)) {
> +	if (flags != 0 || (saddr_low & (SZ_16 - 1)) || num_events == 0 ||
> +	    check_mul_overflow(num_events, sizeof(*einfo), &shmem_size)) {

[Severity: High]
Is there a missing upper bound check for num_events here?

While check_mul_overflow() prevents integer wrap-around, it still allows a
guest to pass extremely large values for num_events (e.g., hundreds of
millions) since size_t is 64-bit on RV64.

>  		ret = SBI_ERR_INVALID_PARAM;
>  		goto out;
>  	}
[ ... ]
> -	einfo = kzalloc(shmem_size, GFP_KERNEL);
> +	einfo = kvcalloc(num_events, sizeof(*einfo),
> +			 GFP_KERNEL_ACCOUNT | __GFP_NOWARN);

[Severity: High]
Can this unbounded allocation lead to host memory exhaustion?

Because the patch replaces kzalloc() with kvcalloc() without adding a
functional upper limit on num_events, an untrusted guest could force the
host to allocate and zero gigabytes of vmalloc memory. Even with
GFP_KERNEL_ACCOUNT, this could cause severe memory pressure or memcg OOMs.

>  	if (!einfo) {
>  		ret = SBI_ERR_FAILURE;
>  		goto out;
>  	}
[ ... ]
> -	for (int i = 0; i < num_events; i++) {
> +	for (unsigned long i = 0; i < num_events; i++) {
>  		eidx = einfo[i].event_idx;
>  		etype = kvm_pmu_get_perf_event_type(eidx);

[Severity: High]
If the guest backs the request with valid memory, could this loop cause a
host soft lockup?

This block iterates num_events times synchronously. For very large inputs,
this could execute millions of times without cond_resched(), monopolizing
the host CPU and causing RCU stalls. Should num_events be bounded to a sane
architectural maximum?

>  		econfig = kvm_pmu_get_perf_event_config(eidx, einfo[i].event_data);
>  		ret = riscv_pmu_get_event_info(etype, econfig, NULL);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730091710.1366757-1-2045gemini@gmail.com?part=1