Hi Julien, Marc,
On 2019/7/17 23:00, Marc Zyngier wrote:
On 17/07/2019 14:44, Julien Thierry wrote:
Hi Zenghui,
On 17/07/2019 13:20, Zenghui Yu wrote:
We use "pmc->idx" and the "chained" bitmap to determine if the pmc is
chained, in kvm_pmu_pmc_is_chained(). But idx might be uninitialized
(and random) when we doing this decision, through a KVM_ARM_VCPU_INIT
ioctl -> kvm_pmu_vcpu_reset(). And the test_bit() against this random
idx will potentially hit a KASAN BUG [1].
Fix it by moving the assignment of idx before kvm_pmu_stop_counter().
[1] https://www.spinics.net/lists/kvm-arm/msg36700.html
Fixes: 80f393a23be6 ("KVM: arm/arm64: Support chained PMU counters")
Suggested-by: Andrew Murray <andrew.murray@xxxxxxx>
Cc: Marc Zyngier <maz@xxxxxxxxxx>
Signed-off-by: Zenghui Yu <yuzenghui@xxxxxxxxxx>> ---
virt/kvm/arm/pmu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
index 3dd8238..521bfdd 100644
--- a/virt/kvm/arm/pmu.c
+++ b/virt/kvm/arm/pmu.c
@@ -225,8 +225,8 @@ void kvm_pmu_vcpu_reset(struct kvm_vcpu *vcpu)
struct kvm_pmu *pmu = &vcpu->arch.pmu;
for (i = 0; i < ARMV8_PMU_MAX_COUNTERS; i++) {
- kvm_pmu_stop_counter(vcpu, &pmu->pmc[i]);
pmu->pmc[i].idx = i;
Yes, this is kind of a static property that should really be part of a
"kvm_pmu_vcpu_init()" or "kvm_pmu_vcpu_create()" and is not expected to
be modified across resets...
There is no such function at the time and I'm unsure whether this
warrants creating that separate function (I would still suggest creating
it to make things clearer).
Yup, that's pretty bad, now that you mention it. I'd be all for the
introduction of kvm_pmu_vcpu_init(), given that we already have
kvm_pmu_vcpu_destroy().
+ kvm_pmu_stop_counter(vcpu, &pmu->pmc[i]);
Whatever other opinions are on splitting pmu_vcpu_init/reset, that
change makes sense and fixes the issue:
Acked-by: Julien Thierry <julien.thierry@xxxxxxx>
}
bitmap_zero(vcpu->arch.pmu.chained, ARMV8_PMU_MAX_COUNTER_PAIRS);
Cheers,
Zenghui, could you please update your patch to take the above into account?
Sure. I will send a v2 with the new subject (may be "KVM: arm/arm64:
Introduce kvm_pmu_vcpu_init() to ...").
Thanks for your suggestions!
zenghui
_______________________________________________
kvmarm mailing list
kvmarm@xxxxxxxxxxxxxxxxxxxxx
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
