qemu-devel
[Top][All Lists]
Advanced

[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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]