[PATCH v1 0/3] RISC-V: KVM: Fix PMU counter handling issues

[PATCH v1 0/3] RISC-V: KVM: Fix PMU counter handling issues

[SeungJu Cheon]

This series fixes three issues in the KVM RISC-V PMU implementation:

- preserve firmware counter values across stop/start;
- report failures when writing snapshot data to guest memory; and
- fix perf-backed counter accounting across read and stop.

The changes were tested with the existing KVM RISC-V SBI PMU selftests
and additional tests covering the affected stop/start, snapshot failure,
and repeated read paths.

SeungJu Cheon (3):
  RISC-V: KVM: Preserve firmware counter value across stop/start
  RISC-V: KVM: Report snapshot write failure to the guest
  RISC-V: KVM: Fix perf-backed counter accounting across stop and read

 arch/riscv/kvm/vcpu_pmu.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)


base-commit: 0a0d1d55dad570724bf8c7ea83409639cfb4be9b
-- 
2.52.0

[PATCH v1 1/3] RISC-V: KVM: Preserve firmware counter value across stop/start

[SeungJu Cheon]

Firmware events accumulate in kvpmu->fw_event[].value while running,
but counter stop only clears fw_event[].started without saving the
value back to pmc->counter_val. A subsequent counter start without
SBI_PMU_START_FLAG_SET_INIT_VALUE reloads the stale counter_val into
fw_event[].value, losing all events counted so far.

Save fw_event[].value into counter_val when actually stopping a
running counter, and remove the now redundant synchronization from
the snapshot path.

Fixes: badc386869e2c ("RISC-V: KVM: Support firmware events")
Signed-off-by: SeungJu Cheon <suunj1331@gmail.com>
---
 arch/riscv/kvm/vcpu_pmu.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c
index 2025b664961c..56dd5b1b2846 100644
--- a/arch/riscv/kvm/vcpu_pmu.c
+++ b/arch/riscv/kvm/vcpu_pmu.c
@@ -671,10 +671,12 @@ int kvm_riscv_vcpu_pmu_ctr_stop(struct kvm_vcpu *vcpu, unsigned long ctr_base,
 				goto out;
 			}
 
-			if (!kvpmu->fw_event[fevent_code].started)
+			if (!kvpmu->fw_event[fevent_code].started) {
 				sbiret = SBI_ERR_ALREADY_STOPPED;
-
-			kvpmu->fw_event[fevent_code].started = false;
+			} else {
+				kvpmu->fw_event[fevent_code].started = false;
+				pmc->counter_val = kvpmu->fw_event[fevent_code].value;
+			}
 		} else if (pmc->perf_event) {
 			if (pmc->started) {
 				/* Stop counting the counter */
@@ -692,9 +694,7 @@ int kvm_riscv_vcpu_pmu_ctr_stop(struct kvm_vcpu *vcpu, unsigned long ctr_base,
 		}
 
 		if (snap_flag_set && !sbiret) {
-			if (pmc->cinfo.type == SBI_PMU_CTR_TYPE_FW)
-				pmc->counter_val = kvpmu->fw_event[fevent_code].value;
-			else if (pmc->perf_event)
+			if (pmc->perf_event)
 				pmc->counter_val += perf_event_read_value(pmc->perf_event,
 									  &enabled, &running);
 			/*
-- 
2.52.0

[PATCH v1 2/3] RISC-V: KVM: Report snapshot write failure to the guest

[SeungJu Cheon]

If kvm_vcpu_write_guest() fails while updating the PMU snapshot area
on counter stop, the guest may receive SBI_SUCCESS without the
snapshot being updated, leaving stale data in shared memory.

Return SBI_ERR_FAILURE when the snapshot write fails.

Fixes: c2f41ddbcdd7 ("RISC-V: KVM: Implement SBI PMU Snapshot feature")
Signed-off-by: SeungJu Cheon <suunj1331@gmail.com>
---
 arch/riscv/kvm/vcpu_pmu.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c
index 56dd5b1b2846..45dd5233340d 100644
--- a/arch/riscv/kvm/vcpu_pmu.c
+++ b/arch/riscv/kvm/vcpu_pmu.c
@@ -723,9 +723,10 @@ int kvm_riscv_vcpu_pmu_ctr_stop(struct kvm_vcpu *vcpu, unsigned long ctr_base,
 		}
 	}
 
-	if (shmem_needs_update)
-		kvm_vcpu_write_guest(vcpu, kvpmu->snapshot_addr, kvpmu->sdata,
-					     sizeof(struct riscv_pmu_snapshot_data));
+	if (shmem_needs_update &&
+	    kvm_vcpu_write_guest(vcpu, kvpmu->snapshot_addr, kvpmu->sdata,
+				 sizeof(struct riscv_pmu_snapshot_data)))
+		sbiret = SBI_ERR_FAILURE;
 
 out:
 	retdata->err_val = sbiret;
-- 
2.52.0

[PATCH v1 3/3] RISC-V: KVM: Fix perf-backed counter accounting across stop and read

[SeungJu Cheon]

pmu_ctr_read() adds the event count returned by perf_event_read_value()
to counter_val, which can accumulate the same count repeatedly across
reads. kvm_riscv_vcpu_pmu_ctr_stop() also leaves counter_val stale by
not folding the current event count into it.

Make reads of perf-backed counters side-effect free, and use
perf_event_pause() when stopping a counter to fold the current event
count into counter_val while resetting it. This preserves the counter
value across stop/start and lets the snapshot path use counter_val
directly.

Fixes: 0cb74b65d2e5 ("RISC-V: KVM: Implement perf support without sampling")
Signed-off-by: SeungJu Cheon <suunj1331@gmail.com>
---
 arch/riscv/kvm/vcpu_pmu.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c
index 45dd5233340d..7d50490cc622 100644
--- a/arch/riscv/kvm/vcpu_pmu.c
+++ b/arch/riscv/kvm/vcpu_pmu.c
@@ -268,12 +268,13 @@ static int pmu_ctr_read(struct kvm_vcpu *vcpu, unsigned long cidx,
 			return -EINVAL;
 
 		pmc->counter_val = kvpmu->fw_event[fevent_code].value;
+		*out_val = pmc->counter_val;
 	} else if (pmc->perf_event) {
-		pmc->counter_val += perf_event_read_value(pmc->perf_event, &enabled, &running);
+		*out_val = pmc->counter_val +
+			   perf_event_read_value(pmc->perf_event, &enabled, &running);
 	} else {
 		return -EINVAL;
 	}
-	*out_val = pmc->counter_val;
 
 	return 0;
 }
@@ -641,7 +642,6 @@ int kvm_riscv_vcpu_pmu_ctr_stop(struct kvm_vcpu *vcpu, unsigned long ctr_base,
 {
 	struct kvm_pmu *kvpmu = vcpu_to_pmu(vcpu);
 	int i, pmc_index, sbiret = 0;
-	u64 enabled, running;
 	struct kvm_pmc *pmc;
 	int fevent_code;
 	bool snap_flag_set = flags & SBI_PMU_STOP_FLAG_TAKE_SNAPSHOT;
@@ -679,8 +679,11 @@ int kvm_riscv_vcpu_pmu_ctr_stop(struct kvm_vcpu *vcpu, unsigned long ctr_base,
 			}
 		} else if (pmc->perf_event) {
 			if (pmc->started) {
-				/* Stop counting the counter */
-				perf_event_disable(pmc->perf_event);
+				/*
+				 * Stop the counter and fold the live count into counter_val.
+				 * Reset the event value to avoid redundant accumulation.
+				 */
+				pmc->counter_val += perf_event_pause(pmc->perf_event, true);
 				pmc->started = false;
 			} else {
 				sbiret = SBI_ERR_ALREADY_STOPPED;
@@ -694,9 +697,6 @@ int kvm_riscv_vcpu_pmu_ctr_stop(struct kvm_vcpu *vcpu, unsigned long ctr_base,
 		}
 
 		if (snap_flag_set && !sbiret) {
-			if (pmc->perf_event)
-				pmc->counter_val += perf_event_read_value(pmc->perf_event,
-									  &enabled, &running);
 			/*
 			 * The counter and overflow indices in the snapshot region are w.r.to
 			 * cbase. Modify the set bit in the counter mask instead of the pmc_index
-- 
2.52.0

Re: [PATCH v1 3/3] RISC-V: KVM: Fix perf-backed counter accounting across stop and read

[sashiko-bot]

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

Pre-existing issues:
- [High] Starting an already-started counter overwrites its internal value before failing, causing subsequent reads to return corrupt data.
- [High] Hardware counters configured with `SBI_PMU_CFG_FLAG_AUTO_START` are not marked as started, preventing them from ever being successfully stopped.
- [High] Reading high 32-bit PMU CSRs (like CSR_CYCLEH) in a 32-bit RISC-V KVM guest results in an illegal instruction trap, breaking hardware performance monitoring on RV32.
- [Medium] The variable `sbiret` accumulates the error state across iterations of the counter mask, causing subsequent valid counters to fail snapshotting if an earlier counter was already stopped.
--

--- Patch [3]: [PATCH v1 3/3] RISC-V: KVM: Fix perf-backed counter accounting across stop and read ---
commit 4a25612420d054353a1af5f2be56663ddf614797
Author: SeungJu Cheon <suunj1331@gmail.com>

RISC-V: KVM: Fix perf-backed counter accounting across stop and read

pmu_ctr_read() adds the event count returned by perf_event_read_value()
to counter_val, which can accumulate the same count repeatedly across
reads. kvm_riscv_vcpu_pmu_ctr_stop() also leaves counter_val stale by
not folding the current event count into it.

Make reads of perf-backed counters side-effect free, and use
perf_event_pause() when stopping a counter to fold the current event
count into counter_val while resetting it. This preserves the counter
value across stop/start and lets the snapshot path use counter_val
directly.

> diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c
> index 45dd5233340dc..7d50490cc6222 100644
> --- a/arch/riscv/kvm/vcpu_pmu.c
> +++ b/arch/riscv/kvm/vcpu_pmu.c
> @@ -268,12 +268,13 @@ static int pmu_ctr_read(struct kvm_vcpu *vcpu, unsigned long cidx,
>  			return -EINVAL;
>  
>  		pmc->counter_val = kvpmu->fw_event[fevent_code].value;
> +		*out_val = pmc->counter_val;
>  	} else if (pmc->perf_event) {
> -		pmc->counter_val += perf_event_read_value(pmc->perf_event, &enabled, &running);
> +		*out_val = pmc->counter_val +
> +			   perf_event_read_value(pmc->perf_event, &enabled, &running);
>  	} else {

[Severity: High]
This isn't a bug introduced by this patch, but does reading high 32-bit
PMU CSRs in a 32-bit RISC-V KVM guest result in an illegal instruction trap?

If an RV32 guest executes a CSR read instruction on a high PMU CSR (like
CSR_CYCLEH), kvm_riscv_vcpu_pmu_read_hpm() calculates the index:

arch/riscv/kvm/vcpu_pmu.c:kvm_riscv_vcpu_pmu_read_hpm() {
    ...
    cidx = csr_num - CSR_CYCLE;

    if (pmu_ctr_read(vcpu, cidx, val) < 0)
        return KVM_INSN_ILLEGAL_TRAP;
    ...
}

For high CSRs, which are offset by 0x80 from the base CSRs on RV32, cidx
would be out of bounds. Will this cause the bounds check at the beginning
of pmu_ctr_read() to fail and reject the high CSR read, breaking hardware
performance monitoring entirely on RV32?

>  		return -EINVAL;
>  	}
> -	*out_val = pmc->counter_val;
>  
>  	return 0;
>  }

[ ... ]

> @@ -679,8 +679,11 @@ int kvm_riscv_vcpu_pmu_ctr_stop(struct kvm_vcpu *vcpu, unsigned long ctr_base,
>  			}
>  		} else if (pmc->perf_event) {
>  			if (pmc->started) {
> -				/* Stop counting the counter */
> -				perf_event_disable(pmc->perf_event);
> +				/*
> +				 * Stop the counter and fold the live count into counter_val.
> +				 * Reset the event value to avoid redundant accumulation.
> +				 */
> +				pmc->counter_val += perf_event_pause(pmc->perf_event, true);
>  				pmc->started = false;
>  			} else {
>  				sbiret = SBI_ERR_ALREADY_STOPPED;
>  			}

[Severity: High]
This is a pre-existing issue, but does pmc->started accurately track the
running state for counters configured with auto-start?

In kvm_pmu_create_perf_event(), the perf event is enabled for AUTO_START
without updating the tracking state pmc->started:

arch/riscv/kvm/vcpu_pmu.c:kvm_pmu_create_perf_event() {
    ...
    if (flags & SBI_PMU_CFG_FLAG_AUTO_START)
        perf_event_enable(pmc->perf_event);
    ...
}

When the guest later tries to stop the counter here in
kvm_riscv_vcpu_pmu_ctr_stop(), will it hit this !pmc->started branch,
skip calling perf_event_pause(), and leak the running state of the counter
on the host?

> @@ -694,9 +697,6 @@ int kvm_riscv_vcpu_pmu_ctr_stop(struct kvm_vcpu *vcpu, unsigned long ctr_base,
>  		}
>  
>  		if (snap_flag_set && !sbiret) {
> -			if (pmc->perf_event)
> -				pmc->counter_val += perf_event_read_value(pmc->perf_event,
> -									  &enabled, &running);

[Severity: Medium]
This is a pre-existing issue, but does sbiret accumulate the error state
across iterations of the counter mask?

If a guest VM calls SBI PMU stop with a counter mask containing multiple
counters, and an earlier counter is already stopped, sbiret is set to
SBI_ERR_ALREADY_STOPPED and never reset.

Will subsequent running counters be successfully stopped, but have their
snapshot update bypassed in shared memory because sbiret remains non-zero?

>  			/*
>  			 * The counter and overflow indices in the snapshot region are w.r.to
>  			 * cbase. Modify the set bit in the counter mask instead of the pmc_index


While looking at this area, I also noticed another pre-existing issue related
to starting counters:

[Severity: High]
This is a pre-existing issue, but does starting an already-started counter
overwrite its internal value before failing?

arch/riscv/kvm/vcpu_pmu.c:kvm_riscv_vcpu_pmu_ctr_start() {
    ...
    if (flags & SBI_PMU_START_FLAG_SET_INIT_VALUE) {
        pmc->counter_val = ival;
    }
    ...
    if (unlikely(pmc->started)) {
        sbiret = SBI_ERR_ALREADY_STARTED;
        continue;
    }
    ...
}

If a guest VM calls SBI PMU start on an already-started counter with the
initialization flag set, pmc->counter_val is overwritten unconditionally
before the pmc->started check aborts the operation.

Could this cause subsequent reads to return corrupt data, allowing a guest
OS to corrupt the internal state machine of its own PMU counters?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825083719.643970-1-suunj1331@gmail.com?part=3

Re: [PATCH v1 1/3] RISC-V: KVM: Preserve firmware counter value across stop/start

[sashiko-bot]

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

Pre-existing issues:
- [Critical] The guest can trigger a host kernel panic and memory leaks by intentionally failing the shared memory setup.
- [High] The `sbiret` error variable is not reset per iteration in the `for_each_set_bit` loop of `kvm_riscv_vcpu_pmu_ctr_stop`, causing subsequent successfully stopped counters to silently drop their snapshot updates.
--

--- Patch [1]: [PATCH v1 1/3] RISC-V: KVM: Preserve firmware counter value across stop/start ---
commit 1ea35d28d13ff19e21fbca9d91839aa79150c9a5
Author: SeungJu Cheon <suunj1331@gmail.com>

RISC-V: KVM: Preserve firmware counter value across stop/start

Firmware events accumulate in kvpmu->fw_event[].value while running,
but counter stop only clears fw_event[].started without saving the
value back to pmc->counter_val. A subsequent counter start without
SBI_PMU_START_FLAG_SET_INIT_VALUE reloads the stale counter_val into
fw_event[].value, losing all events counted so far.

> diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c
> index 2025b664961c8..56dd5b1b2846c 100644
> --- a/arch/riscv/kvm/vcpu_pmu.c
> +++ b/arch/riscv/kvm/vcpu_pmu.c
> @@ -671,10 +671,12 @@ int kvm_riscv_vcpu_pmu_ctr_stop(struct kvm_vcpu *vcpu, unsigned long ctr_base,
>  				goto out;
>  			}
>  
> -			if (!kvpmu->fw_event[fevent_code].started)
> +			if (!kvpmu->fw_event[fevent_code].started) {
>  				sbiret = SBI_ERR_ALREADY_STOPPED;
> -
> -			kvpmu->fw_event[fevent_code].started = false;
> +			} else {
> +				kvpmu->fw_event[fevent_code].started = false;
> +				pmc->counter_val = kvpmu->fw_event[fevent_code].value;
> +			}
>  		} else if (pmc->perf_event) {
>  			if (pmc->started) {
>  				/* Stop counting the counter */
> @@ -692,9 +694,7 @@ int kvm_riscv_vcpu_pmu_ctr_stop(struct kvm_vcpu *vcpu, unsigned long ctr_base,
>  		}
>  
>  		if (snap_flag_set && !sbiret) {

[Severity: High]
This isn't a bug introduced by this patch, but is it possible for sbiret to
remain set for subsequent iterations of the loop?

Looking at kvm_riscv_vcpu_pmu_ctr_stop() in arch/riscv/kvm/vcpu_pmu.c, if the
guest provides a counter mask with multiple counters and an early counter is
already stopped, sbiret is set to SBI_ERR_ALREADY_STOPPED.

Because sbiret isn't reset at the beginning of each loop iteration in the
for_each_set_bit(i, &ctr_mask, BITS_PER_LONG) loop, subsequent running counters
in the mask will be stopped, but the condition if (snap_flag_set && !sbiret)
will evaluate to false.

Does this cause successfully stopped counters to silently skip their snapshot
updates?

> -			if (pmc->cinfo.type == SBI_PMU_CTR_TYPE_FW)
> -				pmc->counter_val = kvpmu->fw_event[fevent_code].value;
> -			else if (pmc->perf_event)
> +			if (pmc->perf_event)
>  				pmc->counter_val += perf_event_read_value(pmc->perf_event,
>  									  &enabled, &running);
>  			/*

[Severity: Critical]
This is a pre-existing issue, but can the guest trigger a host kernel panic
here if it intentionally fails the shared memory setup?

If the guest calls SBI_EXT_PMU_SNAPSHOT_SET_SHMEM with a valid GPA,
kvpmu->snapshot_addr is set and kvpmu->sdata is allocated.

If the guest calls it again with an unmapped GPA:

arch/riscv/kvm/vcpu_pmu.c:kvm_riscv_vcpu_pmu_snapshot_set_shmem() {
    ...
    kvpmu->sdata = kzalloc(snapshot_area_size, GFP_ATOMIC);
    ...
    if (kvm_vcpu_write_guest(vcpu, shmem_base, kvpmu->sdata,
                             snapshot_area_size)) {
        kfree(kvpmu->sdata);
        kvpmu->sdata = NULL;
        sbiret = SBI_ERR_INVALID_ADDRESS;
        goto out;
    }
    ...
}

This unconditionally allocates a new kvpmu->sdata without freeing the old one,
causing a memory leak. Since kvm_vcpu_write_guest() fails, kvpmu->sdata is
freed and set to NULL, and the goto out bypasses updating kvpmu->snapshot_addr.

If the guest then calls SBI_EXT_PMU_COUNTER_STOP with
SBI_PMU_STOP_FLAG_TAKE_SNAPSHOT, kvpmu->snapshot_addr is still the old valid
GPA. The stop operation bypasses the INVALID_GPA check in
kvm_riscv_vcpu_pmu_ctr_stop() and eventually executes:

        if (snap_flag_set && !sbiret) {
            ...
            kvpmu->sdata->ctr_values[i] = pmc->counter_val;

Will this dereference the NULL kvpmu->sdata pointer and cause a panic?

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

Re: [PATCH v1 2/3] RISC-V: KVM: Report snapshot write failure to the guest

[sashiko-bot]

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

Pre-existing issues:
- [Critical] Repeated or failed calls to SBI_EXT_PMU_SNAPSHOT_SET_SHMEM cause a memory leak and lead to a NULL pointer dereference.
- [Critical] Integer overflow in kvm_riscv_vcpu_pmu_event_info causes a massive heap buffer overflow.
- [High] A failure while stopping one PMU counter prevents snapshot updates for all subsequent successfully stopped counters.
- [High] Starting an already-started counter silently corrupts its base value and violates state atomicity.
--

--- Patch [2]: [PATCH v1 2/3] RISC-V: KVM: Report snapshot write failure to the guest ---
commit 2b7f150df4c06e5c182586d3b92e176b5a3ac870
Author: SeungJu Cheon <suunj1331@gmail.com>

RISC-V: KVM: Report snapshot write failure to the guest

If kvm_vcpu_write_guest() fails while updating the PMU snapshot area
on counter stop, the guest may receive SBI_SUCCESS without the
snapshot being updated, leaving stale data in shared memory. This
commit ensures SBI_ERR_FAILURE is returned when the write fails.

> diff --git a/arch/riscv/kvm/vcpu_pmu.c b/arch/riscv/kvm/vcpu_pmu.c
> index 56dd5b1b2846c..45dd5233340dc 100644
> --- a/arch/riscv/kvm/vcpu_pmu.c
> +++ b/arch/riscv/kvm/vcpu_pmu.c
> @@ -723,9 +723,10 @@ int kvm_riscv_vcpu_pmu_ctr_stop(struct kvm_vcpu *vcpu, unsigned long ctr_base,
>  		}
>  	}
>  
> -	if (shmem_needs_update)
> -		kvm_vcpu_write_guest(vcpu, kvpmu->snapshot_addr, kvpmu->sdata,
> -					     sizeof(struct riscv_pmu_snapshot_data));
> +	if (shmem_needs_update &&
> +	    kvm_vcpu_write_guest(vcpu, kvpmu->snapshot_addr, kvpmu->sdata,
> +				 sizeof(struct riscv_pmu_snapshot_data)))
> +		sbiret = SBI_ERR_FAILURE;
>  
>  out:
>  	retdata->err_val = sbiret;

[Severity: High]
This isn't a bug introduced by this patch, but looking at the loop earlier in
kvm_riscv_vcpu_pmu_ctr_stop():

	for_each_set_bit(i, &ctr_mask, BITS_PER_LONG) {
		/* ... */
		if (snap_flag_set && !sbiret) {
			/* ... */
			kvpmu->sdata->ctr_values[i] = pmc->counter_val;
			shmem_needs_update = true;
		}

Does a failure while stopping one PMU counter prevent snapshot updates for all
subsequent successfully stopped counters? 

The loop relies on sbiret being zero to update the snapshot, but sbiret acts
as a global error flag that is never cleared between iterations.


[Severity: High]
This is also a pre-existing issue, but I noticed a potential concern in
kvm_riscv_vcpu_pmu_ctr_start():

	for_each_set_bit(i, &ctr_mask, BITS_PER_LONG) {
		/* ... */
		pmc = &kvpmu->pmc[pmc_index];
		if (flags & SBI_PMU_START_FLAG_SET_INIT_VALUE) {
			pmc->counter_val = ival;
		} /* ... */

		} else if (pmc->perf_event) {
			if (unlikely(pmc->started)) {
				sbiret = SBI_ERR_ALREADY_STARTED;
				continue;
			}

Does starting an already-started counter silently corrupt its base value?

The pmc->counter_val is modified before validating the pmc->started state,
which could permanently overwrite the active counter base value.


[Severity: Critical]
This is a pre-existing issue, but can repeated or failed calls to
SBI_EXT_PMU_SNAPSHOT_SET_SHMEM cause a memory leak and lead to a NULL
pointer dereference?

In kvm_riscv_vcpu_pmu_snapshot_set_shmem():

	kvpmu->sdata = kzalloc(snapshot_area_size, GFP_ATOMIC);
	if (!kvpmu->sdata) {
		sbiret = SBI_ERR_FAILURE;
		goto out;
	}

	/* No need to check writable slot explicitly as kvm_vcpu_write_guest does it internally */
	if (kvm_vcpu_write_guest(vcpu, saddr, kvpmu->sdata, snapshot_area_size)) {
		kfree(kvpmu->sdata);
		kvpmu->sdata = NULL;
		sbiret = SBI_ERR_INVALID_ADDRESS;
		goto out;
	}

It unconditionally overwrites kvpmu->sdata without freeing the previous
allocation. Additionally, upon error, it sets kvpmu->sdata to NULL without
clearing kvpmu->snapshot_addr.

Later, in kvm_riscv_vcpu_pmu_ctr_stop():

		if (snap_flag_set && !sbiret) {
			/* ... */
			if (test_bit(pmc_index, kvpmu->pmc_overflown))
				kvpmu->sdata->ctr_overflow_mask |= BIT(i);
			kvpmu->sdata->ctr_values[i] = pmc->counter_val;

The mismatch between a valid snapshot_addr and a NULL sdata could cause a
direct NULL pointer dereference here.


[Severity: Critical]
This isn't a bug introduced by this patch, but does the shmem_size calculation
in kvm_riscv_vcpu_pmu_event_info() overflow?

	struct riscv_pmu_event_info *einfo = NULL;
	int shmem_size = num_events * sizeof(*einfo);
	/* ... */
	einfo = kzalloc(shmem_size, GFP_KERNEL);
	/* ... */
	for (int i = 0; i < num_events; i++) {
		/* ... */
		einfo[i].output = (ret > 0) ? 1 : 0;
	}

The integer truncation of shmem_size results in a small allocation when
num_events is very large (e.g., 0x10000001), while the loop uses the
untruncated num_events. 

Can this cause out-of-bounds heap writes during the loop execution?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260825083719.643970-1-suunj1331@gmail.com?part=2