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

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

[Zongmin Zhou]

From: Zongmin Zhou <zhouzongmin@kylinos.cn>

A guest may call SBI_PMU_SNAPSHOT_SET_SHMEM repeatedly. Each call
overwrites kvpmu->sdata without freeing the old buffer (memory leak),
and if a later kvm_vcpu_write_guest() fails, the error path frees
sdata but leaves snapshot_addr stale. A subsequent
SBI_PMU_COUNTER_START then passes the INVALID_GPA check and crashes
the host with a NULL buffer in kvm_vcpu_read_guest().

Fix this by clearing the previously installed snapshot area before
installing a new one, which keeps sdata and snapshot_addr consistent.
The SBI spec suggests a single invocation but defines no error code
for repeated calls, so KVM must tolerate them.

Fixes: c2f41ddbcdd756 ("RISC-V: KVM: Implement SBI PMU Snapshot feature")
Cc: stable@vger.kernel.org
Signed-off-by: Zongmin Zhou <zhouzongmin@kylinos.cn>
---
 arch/riscv/kvm/vcpu_pmu.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c
index 752f1014d633e7d67f69c27b72088ed560c187ac..14f73490f12cbc5a8547859f0d1bb1170d89403b 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) {
 		sbiret = SBI_ERR_FAILURE;
-- 
2.34.1


No virus found
		Checked by Hillstone Network AntiVirus

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

[sashiko-bot]

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