[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[RFC PATCH 1/1] kvm-all.c: hint Valgrind that kvm_get_one_reg() inits me
From: |
Daniel Henrique Barboza |
Subject: |
[RFC PATCH 1/1] kvm-all.c: hint Valgrind that kvm_get_one_reg() inits memory |
Date: |
Tue, 5 Apr 2022 10:04:39 -0300 |
There is a lot of Valgrind warnings about conditional jump depending on
unintialized values like this one (taken from a pSeries guest):
Conditional jump or move depends on uninitialised value(s)
at 0xB011DC: kvmppc_enable_cap_large_decr (kvm.c:2544)
by 0x92F28F: cap_large_decr_cpu_apply (spapr_caps.c:523)
by 0x930C37: spapr_caps_cpu_apply (spapr_caps.c:921)
by 0x955D3B: spapr_reset_vcpu (spapr_cpu_core.c:73)
(...)
Uninitialised value was created by a stack allocation
at 0xB01150: kvmppc_enable_cap_large_decr (kvm.c:2538)
In this case, the alleged unintialized value is the 'lpcr' variable that
is written by kvm_get_one_reg() and then used in an if clause:
int kvmppc_enable_cap_large_decr(PowerPCCPU *cpu, int enable)
{
CPUState *cs = CPU(cpu);
uint64_t lpcr;
kvm_get_one_reg(cs, KVM_REG_PPC_LPCR_64, &lpcr);
/* Do we need to modify the LPCR? */
if (!!(lpcr & LPCR_LD) != !!enable) { <---- Valgrind warns here
(...)
A quick fix is to init the variable that kvm_get_one_reg() is going to
write ('lpcr' in the example above). Another idea is to convince
Valgrind that kvm_get_one_reg() inits the 'void *target' memory in case
the ioctl() is successful. This will put some boilerplate in the
function but it will bring benefit for its other callers.
This patch uses the memcheck VALGRING_MAKE_MEM_DEFINED() to mark the
'target' variable as initialized if the ioctl is successful.
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Daniel Henrique Barboza <danielhb413@gmail.com>
---
accel/kvm/kvm-all.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 5f1377ca04..d9acba23c7 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -53,6 +53,10 @@
#include <sys/eventfd.h>
#endif
+#ifdef CONFIG_VALGRIND_H
+#include <valgrind/memcheck.h>
+#endif
+
/* KVM uses PAGE_SIZE in its definition of KVM_COALESCED_MMIO_MAX. We
* need to use the real host PAGE_SIZE, as that's what KVM will use.
*/
@@ -3504,6 +3508,19 @@ int kvm_get_one_reg(CPUState *cs, uint64_t id, void
*target)
if (r) {
trace_kvm_failed_reg_get(id, strerror(-r));
}
+
+#ifdef CONFIG_VALGRIND_H
+ if (r == 0) {
+ switch (id & KVM_REG_SIZE_MASK) {
+ case KVM_REG_SIZE_U32:
+ VALGRIND_MAKE_MEM_DEFINED(target, sizeof(uint32_t));
+ break;
+ case KVM_REG_SIZE_U64:
+ VALGRIND_MAKE_MEM_DEFINED(target, sizeof(uint64_t));
+ break;
+ }
+ }
+#endif
return r;
}
--
2.35.1