riscv-software-src / riscv-software-src/opensbi
PMU: guard num_events multiplication in sbi_pmu_event_get_info()
Nobody has claimed this yet.
- Dominant language
- C
- Stars
- 1.5k
- Forks
- 712
- PR merge metrics
- No merged PRs in 30d
Description
Problem
sbi_pmu_event_get_info() computes the shared-memory size from the caller-provided num_events value:
unsigned long shmem_size = num_events * sizeof(struct sbi_pmu_event_info);
The function rejects num_events == 0, but it does not check whether this multiplication overflows. The computed shmem_size is then used for the range check and mapping, while the later loop still iterates using the original num_events value.
On RV64, a sufficiently large non-zero num_events can make the byte-size calculation wrap, so the validated and mapped range can be smaller than the range accessed by the loop.
Observed result
In a local QEMU virt test built from the revision above, a normal control case returned successfully. The overflow case then hit an M-mode trap and did not return before timeout.
Suggested fix
Reject counts that would overflow the byte-size calculation before checking or mapping the shared-memory range, for example:
if (!num_events ||
num_events > ULONG_MAX / sizeof(struct sbi_pmu_event_info))
return SBI_ERR_INVALID_PARAM;
Scope
I have only validated the firmware fault/hang behavior on QEMU. I am not claiming a broader impact beyond this memory-safety/availability issue.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start at sbi_pmu_event_get_info() and inspect how num_events is used to calculate the shared-memory size before range checking and mapping. Reproduce the overflow case on the QEMU virt test described in the issue; done means oversized non-zero counts are rejected with SBI_ERR_INVALID_PARAM without reaching the later loop.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 78/100