Re: [PATCH] RISC-V: KVM: Fix sdata leak and stale snapshot_addr in snapshot_set_shmem

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Guest-triggerable `pr_warn` allows a malicious guest to spam the host kernel log.
--

commit 26a57cbd434dd543af868b0052521f7a8fda7faa
Author: Zongmin Zhou <zhouzongmin@kylinos.cn>

RISC-V: KVM: Fix sdata leak and stale snapshot_addr in snapshot_set_shmem

A guest may call SBI_PMU_SNAPSHOT_SET_SHMEM repeatedly. Each call overwrites
kvpmu->sdata without freeing the old buffer, and if a later guest write
fails, the error path frees sdata but leaves snapshot_addr stale. This
clears the previously installed snapshot area before installing a new one.

> diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c
> index 752f1014d633e..14f73490f12cb 100644
> --- a/arch/riscv/kvm/vcpu_pmu.c
> +++ b/arch/riscv/kvm/vcpu_pmu.c
> @@ -454,6 +454,14 @@ int kvm_riscv_vcpu_pmu_snapshot_set_shmem(struct kvm_vcpu *vcpu, unsigned long s
>  		}
>  	}
>  
> +	/*
> +	 * Clear any previously installed snapshot area to avoid leaking
> +	 * the old sdata and to keep sdata/snapshot_addr consistent if
> +	 * the re-install fails below.
> +	 */
> +	if (kvpmu->snapshot_addr != INVALID_GPA)
> +		kvm_pmu_clear_snapshot_area(vcpu);
> +
>  	kvpmu->sdata = kzalloc(snapshot_area_size, GFP_ATOMIC | __GFP_ACCOUNT);
>  	if (!kvpmu->sdata) {

[Severity: High]
This isn't a bug introduced by this patch, but while reviewing the PMU
snapshot code I noticed a pre-existing issue in the same file.

In kvm_riscv_vcpu_pmu_ctr_start(), a guest can intentionally trigger a host
kernel log spam if the shared memory becomes invalid:

        if (kvm_vcpu_read_guest(vcpu, kvpmu->snapshot_addr, kvpmu->sdata,
                                sizeof(struct riscv_pmu_snapshot_data))) {
                pr_warn("Unable to read snapshot shared memory while starting counters\n");
                sbiret = SBI_ERR_FAILURE;
                goto out;
        }

Because this pr_warn() is unconditional and not rate-limited, a guest can
configure a valid snapshot area, invalidate the backing memory, and repeatedly
call SBI_EXT_PMU_COUNTER_START to spam the host log, leading to a denial of
service.

Should this pr_warn() be converted to a rate-limited print, or perhaps dropped
since the error is already returned to the guest?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826064117.58029-1-min_halo@163.com?part=1