RISC-V: KVM: Fix PMU event info array size overflow
Subsystems (3)
PATCH RESEND v2 1/1 RISC-V: KVM: Fix PMU event info array size overflow
SBI PMU EVENT_GET_INFO stores guest-controllednum_events * sizeof(*einfo) in an int. On RV64, num_events = 0x10000001 makes 0x100000010 truncate to 16. KVM then allocates one entry but loops over the original num_events, causing out-of-bounds reads and writes. A nested guest triggered:
BUG: KASAN: slab-out-of-bounds in kvm_riscv_vcpu_pmu_event_info+0xa4/0x142 Read of size 4 at addr ff600000074d46b0 by task init/1 Call Trace: [<ffffffff8006471c>] kvm_riscv_vcpu_pmu_event_info+0xa4/0x142 [<ffffffff800690c0>] kvm_sbi_ext_pmu_handler+0xca/0x268 [<ffffffff8006779e>] kvm_riscv_vcpu_sbi_ecall+0xec/0x1e6 [<ffffffff8006008c>] kvm_riscv_vcpu_exit+0x48c/0x540 [<ffffffff8005ea0a>] kvm_arch_vcpu_ioctl_run+0x37e/0xc80 Allocated by task 1: __kmalloc_noprof+0x19e/0x4b0 kvm_riscv_vcpu_pmu_event_info+0x72/0x142 kvm_sbi_ext_pmu_handler+0xca/0x268 kvm_riscv_vcpu_sbi_ecall+0xec/0x1e6 kvm_riscv_vcpu_exit+0x48c/0x540 kvm_arch_vcpu_ioctl_run+0x37e/0xc80
The buggy address is located 0 bytes to the right of
allocated 16-byte region [ff600000074d46a0, ff600000074d46b0)
Store the shared-memory size in size_t and reject multiplication overflow. Allocate the guest-driven array with GFP_KERNEL_ACCOUNT so it is charged to kmemcg, and use __GFP_NOWARN to suppress allocation failure warnings. Use kvcalloc() to allow vmalloc fallback and an unsigned long loop index to match num_events.
---
RESEND:
- Send v2 as a new top-level thread instead of replying to v1.
Changes in v2:
- Use an accounted, no-warning kvcalloc() allocation and kvfree(), as suggested by Sashiko.
- Naveed Khan reported the issue before v1. I missed his report at the time, so add Reported-by and Closes tags to credit him.
Link to v1: https://lore.kernel.org/kvm/[email protected]/
--- arch/riscv/kvm/vcpu_pmu.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)
arch/riscv/kvm/vcpu_pmu.c +9 -5
@@ -12,7 +12,9 @@#include <linux/err.h> #include <linux/kvm_host.h> #include <linux/nospec.h> +#include <linux/overflow.h> #include <linux/perf/riscv_pmu.h> +#include <linux/slab.h> #include <asm/csr.h> #include <asm/kvm_isa.h> #include <asm/kvm_vcpu_sbi.h>
@@ -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;u32 eidx, etype;u64 econfig;int ret; -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)) { ret =SBI_ERR_INVALID_PARAM ; goto out; }
@@ -500,7 +503,8 @@ int kvm_riscv_vcpu_pmu_event_info(struct kvm_vcpu *vcpu, unsigned long saddr_low } } - einfo =kzalloc (shmem_size,GFP_KERNEL ); + einfo =kvcalloc (num_events,sizeof (*einfo), +GFP_KERNEL_ACCOUNT | __GFP_NOWARN);if (!einfo) { ret =SBI_ERR_FAILURE ; goto out;
@@ -512,7 +516,7 @@ int kvm_riscv_vcpu_pmu_event_info(struct kvm_vcpu *vcpu, unsigned long saddr_low goto free_mem; } -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); econfig =kvm_pmu_get_perf_event_config (eidx, einfo[i].event_data);
@@ -525,7 +529,7 @@ int kvm_riscv_vcpu_pmu_event_info(struct kvm_vcpu *vcpu, unsigned long saddr_low ret =SBI_ERR_INVALID_ADDRESS ; free_mem: -kfree (einfo); +kvfree (einfo); out: retdata->err_val = ret;
Signature · 1 line
2.43.0
[email protected]
AI reviewThank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Unbounded memory allocation and long execution loop due to unvalidated guest-controlled…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unbounded memory allocation and long execution loop due to unvalidated guest-controlled num_events. --
--- Patch [1]: [PATCH v2 RESEND] RISC-V: KVM: Fix PMU event info array size overflow --- commit bdf470babff9670998ba5d0cae1674a0cc0b80b2
Author: Guidong Han <[email protected]>
RISC-V: KVM: Fix PMU event info array size overflow
This commit addresses an integer overflow in the SBI PMU event info array allocation. It switches the allocation to kvcalloc with GFP_KERNEL_ACCOUNT and uses size_t and unsigned long variables to match the guest-provided size without truncation.
Quoted 5 lines
arch/riscv/kvm/vcpu_pmu.c +0
-0@@ -479,13 +481,14 @@ int kvm_riscv_vcpu_pmu_event_info(struct kvm_vcpu *vcpu, unsigned long saddr_low
[ ... ]
Quoted 6 lines
arch/riscv/kvm/vcpu_pmu.c +2
-1-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)) { ret =SBI_ERR_INVALID_PARAM ; goto out; }
[ ... ]
arch/riscv/kvm/vcpu_pmu.c int kvm_riscv_vcpu_pmu_event_info(struct kvm_vcpu *vcpu, unsigned long saddr_low · line 507
Quoted 3 lines
arch/riscv/kvm/vcpu_pmu.c +2
-1- einfo =kzalloc (shmem_size,GFP_KERNEL ); + einfo =kvcalloc (num_events,sizeof (*einfo), +GFP_KERNEL_ACCOUNT | __GFP_NOWARN);
[Severity: High]
Can this introduce an unbounded memory allocation regression based on a guest-controlled num_events?
While check_mul_overflow() prevents integer overflow during the calculation, there doesn't appear to be a check capping num_events to a reasonable architectural maximum. A guest passing an extremely large value could cause kvcalloc() to attempt an allocation approaching the host's total RAM, which could lead to severe host memory thrashing or direct reclaim.
[ ... ]
arch/riscv/kvm/vcpu_pmu.c int kvm_riscv_vcpu_pmu_event_info(struct kvm_vcpu *vcpu, unsigned long saddr_low · line 521
Quoted 4 lines
arch/riscv/kvm/vcpu_pmu.c +1
-1-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]
Could this loop iteration trigger a regression resulting in host soft lockups or RCU stalls?
If a massive allocation from the earlier kvcalloc() succeeds, this loop will iterate num_events times synchronously without yielding. This could allow an untrusted guest to monopolize the host CPU.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Quoted 31 lines · Guidong Han
SBI PMU
EVENT_GET_INFOstores guest-controllednum_events*sizeof(*einfo)in an int. On RV64,num_events=0x10000001makes0x100000010truncate to 16. KVM then allocates one entry but loops over the originalnum_events, causing out-of-bounds reads and writes. A nested guest triggered:BUG: KASAN: slab-out-of-bounds in kvm_riscv_vcpu_pmu_event_info+0xa4/0x142 Read of size 4 at addr ff600000074d46b0 by task init/1 Call Trace: [<ffffffff8006471c>] kvm_riscv_vcpu_pmu_event_info+0xa4/0x142 [<ffffffff800690c0>] kvm_sbi_ext_pmu_handler+0xca/0x268 [<ffffffff8006779e>] kvm_riscv_vcpu_sbi_ecall+0xec/0x1e6 [<ffffffff8006008c>] kvm_riscv_vcpu_exit+0x48c/0x540 [<ffffffff8005ea0a>] kvm_arch_vcpu_ioctl_run+0x37e/0xc80 Allocated by task 1: __kmalloc_noprof+0x19e/0x4b0 kvm_riscv_vcpu_pmu_event_info+0x72/0x142 kvm_sbi_ext_pmu_handler+0xca/0x268 kvm_riscv_vcpu_sbi_ecall+0xec/0x1e6 kvm_riscv_vcpu_exit+0x48c/0x540 kvm_arch_vcpu_ioctl_run+0x37e/0xc80The buggy address is located 0 bytes to the right of
allocated 16-byte region [ff600000074d46a0,ff600000074d46b0)Store the shared-memory size in
size_tand reject multiplication overflow. Allocate the guest-driven array withGFP_KERNEL_ACCOUNTso it is charged to kmemcg, and use__GFP_NOWARNto suppress allocation failure warnings. Usekvcalloc()to allow vmalloc fallback and an unsigned long loop index to matchnum_events.
Quoted 5 lines
Reported-by Naveed Khan <[email protected]>Fixes e309fd113b9f ("RISC-V: KVM: Implement get event info function")Signed-off-by Guidong Han <[email protected]>
LGTM.
Queued this patch for Linux-7.3
Thanks,
Anup
Quoted 76 lines
---
RESEND:
- Send v2 as a new top-level thread instead of replying to v1.Changes in v2:
- Use an accounted, no-warningkvcalloc()allocation andkvfree(), as suggested by Sashiko.
- Naveed Khan reported the issue before v1. I missed his report at the time, so add Reported-by and Closes tags to credit him.Link to v1: https://lore.kernel.org/kvm/[email protected]/
--- arch/riscv/kvm/vcpu_pmu.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-)arch/riscv/kvm/vcpu_pmu.c +9
-6@@ -12,7 +12,9 @@#include <linux/err.h> #include <linux/kvm_host.h> #include <linux/nospec.h> +#include <linux/overflow.h> #include <linux/perf/riscv_pmu.h> +#include <linux/slab.h> #include <asm/csr.h> #include <asm/kvm_isa.h> #include <asm/kvm_vcpu_sbi.h> @@ -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;u32 eidx, etype;u64 econfig;int ret; -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)) { ret =SBI_ERR_INVALID_PARAM ; goto out; }@@ -500,7 +503,8 @@ int kvm_riscv_vcpu_pmu_event_info(struct kvm_vcpu *vcpu, unsigned long saddr_low } } - einfo =kzalloc (shmem_size,GFP_KERNEL ); + einfo =kvcalloc (num_events,sizeof (*einfo), +GFP_KERNEL_ACCOUNT | __GFP_NOWARN);if (!einfo) { ret =SBI_ERR_FAILURE ; goto out;@@ -512,7 +516,7 @@ int kvm_riscv_vcpu_pmu_event_info(struct kvm_vcpu *vcpu, unsigned long saddr_low goto free_mem; } -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); econfig =kvm_pmu_get_perf_event_config (eidx, einfo[i].event_data);@@ -525,7 +529,7 @@ int kvm_riscv_vcpu_pmu_event_info(struct kvm_vcpu *vcpu, unsigned long saddr_low ret =SBI_ERR_INVALID_ADDRESS ; free_mem: -kfree (einfo); +kvfree (einfo); out: retdata->err_val = ret; --2.43.0