From e4adb09f7952fd37b7f1ba3df377d54d0823e682 Mon Sep 17 00:00:00 2001 From: Vitaly Kuznetsov Date: Fri, 16 Jul 2021 13:58:51 +0200 Subject: [PATCH 1/7] i386: assert 'cs->kvm_state' is not null Coverity reports potential NULL pointer dereference in get_supported_hv_cpuid_legacy() when 'cs->kvm_state' is NULL. While 'cs->kvm_state' can indeed be NULL in hv_cpuid_get_host(), kvm_hyperv_expand_features() makes sure that it only happens when KVM_CAP_SYS_HYPERV_CPUID is supported and KVM_CAP_SYS_HYPERV_CPUID implies KVM_CAP_HYPERV_CPUID so get_supported_hv_cpuid_legacy() is never really called. Add asserts to strengthen the protection against broken KVM behavior. Coverity: CID 1458243 Signed-off-by: Vitaly Kuznetsov Message-Id: <20210716115852.418293-1-vkuznets@redhat.com> Signed-off-by: Paolo Bonzini --- target/i386/kvm/kvm.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/target/i386/kvm/kvm.c b/target/i386/kvm/kvm.c index 59ed8327ac..e69abe48e3 100644 --- a/target/i386/kvm/kvm.c +++ b/target/i386/kvm/kvm.c @@ -974,6 +974,12 @@ static struct kvm_cpuid2 *get_supported_hv_cpuid(CPUState *cs) do_sys_ioctl = kvm_check_extension(kvm_state, KVM_CAP_SYS_HYPERV_CPUID) > 0; + /* + * Non-empty KVM context is needed when KVM_CAP_SYS_HYPERV_CPUID is + * unsupported, kvm_hyperv_expand_features() checks for that. + */ + assert(do_sys_ioctl || cs->kvm_state); + /* * When the buffer is too small, KVM_GET_SUPPORTED_HV_CPUID fails with * -E2BIG, however, it doesn't report back the right size. Keep increasing @@ -1105,6 +1111,14 @@ static uint32_t hv_cpuid_get_host(CPUState *cs, uint32_t func, int reg) if (kvm_check_extension(kvm_state, KVM_CAP_HYPERV_CPUID) > 0) { cpuid = get_supported_hv_cpuid(cs); } else { + /* + * 'cs->kvm_state' may be NULL when Hyper-V features are expanded + * before KVM context is created but this is only done when + * KVM_CAP_SYS_HYPERV_CPUID is supported and it implies + * KVM_CAP_HYPERV_CPUID. + */ + assert(cs->kvm_state); + cpuid = get_supported_hv_cpuid_legacy(cs); } hv_cpuid_cache = cpuid; From 14833e24dea49303ebc2464813601054b6cdfcac Mon Sep 17 00:00:00 2001 From: Alexey Neyman Date: Wed, 21 Jul 2021 19:08:46 -0700 Subject: [PATCH 2/7] Makefile: ignore long options When searching for options like -n in MAKEFLAGS, current code may result in a false positive match when make is invoked with long options like --no-print-directory. This has been observed with certain versions of host make (e.g. 3.82) while building the Qemu package in buildroot. Filter out such long options before searching for one-character options. Signed-off-by: Alexey Neyman Message-Id: <20210722020846.3678817-1-stilor@att.net> Signed-off-by: Paolo Bonzini --- Makefile | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 6c36330eef..401c623a65 100644 --- a/Makefile +++ b/Makefile @@ -129,9 +129,11 @@ endif # 4. Rules to bridge to other makefiles ifneq ($(NINJA),) -MAKE.n = $(findstring n,$(firstword $(MAKEFLAGS))) -MAKE.k = $(findstring k,$(firstword $(MAKEFLAGS))) -MAKE.q = $(findstring q,$(firstword $(MAKEFLAGS))) +# Filter out long options to avoid flags like --no-print-directory which +# may result in false positive match for MAKE.n +MAKE.n = $(findstring n,$(firstword $(filter-out --%,$(MAKEFLAGS)))) +MAKE.k = $(findstring k,$(firstword $(filter-out --%,$(MAKEFLAGS)))) +MAKE.q = $(findstring q,$(firstword $(filter-out --%,$(MAKEFLAGS)))) MAKE.nq = $(if $(word 2, $(MAKE.n) $(MAKE.q)),nq) NINJAFLAGS = $(if $V,-v) $(if $(MAKE.n), -n) $(if $(MAKE.k), -k0) \ $(filter-out -j, $(lastword -j1 $(filter -l% -j%, $(MAKEFLAGS)))) \ From 5b945f23d651a71aa722cc6af84a480d41bc549a Mon Sep 17 00:00:00 2001 From: Richard Henderson Date: Mon, 19 Jul 2021 10:01:12 -1000 Subject: [PATCH 3/7] configure: Add -Werror to avx2, avx512 tests When using clang, we get ERROR: configure test passed without -Werror but failed with -Werror. This is probably a bug in the configure script. The failing command will be at the bottom of config.log. You can run configure with --disable-werror to bypass this check. What we really want from these two tests is whether the entire code sequence is supported, including pragmas. Adding -Werror makes the test properly fail for clang. Signed-off-by: Richard Henderson Message-Id: <20210719200112.295316-1-richard.henderson@linaro.org> Signed-off-by: Paolo Bonzini --- configure | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/configure b/configure index 79e2ddc74e..422a456f0b 100755 --- a/configure +++ b/configure @@ -3881,7 +3881,7 @@ static int bar(void *a) { } int main(int argc, char *argv[]) { return bar(argv[0]); } EOF - if compile_object "" ; then + if compile_object "-Werror" ; then avx2_opt="yes" else avx2_opt="no" @@ -3911,7 +3911,7 @@ int main(int argc, char *argv[]) return bar(argv[0]); } EOF - if ! compile_object "" ; then + if ! compile_object "-Werror" ; then avx512f_opt="no" fi else From eceb4f01123355a7045ec4ba9cd547511682a4d9 Mon Sep 17 00:00:00 2001 From: Lara Lazier Date: Sun, 25 Jul 2021 11:08:55 +0200 Subject: [PATCH 4/7] target/i386: Added consistency checks for event injection VMRUN exits with SVM_EXIT_ERR if either: * The event injected has a reserved type. * When the event injected is of type 3 (exception), and the vector that has been specified does not correspond to an exception. This does not fix the entire exc_inj test in kvm-unit-tests. Signed-off-by: Lara Lazier Message-Id: <20210725090855.19713-1-laramglazier@gmail.com> Signed-off-by: Paolo Bonzini --- target/i386/tcg/sysemu/svm_helper.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/target/i386/tcg/sysemu/svm_helper.c b/target/i386/tcg/sysemu/svm_helper.c index 145511d635..90a9de30f8 100644 --- a/target/i386/tcg/sysemu/svm_helper.c +++ b/target/i386/tcg/sysemu/svm_helper.c @@ -383,6 +383,9 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend) cpu_loop_exit(cs); break; case SVM_EVTINJ_TYPE_EXEPT: + if (vector == EXCP02_NMI || vector >= 31) { + cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC()); + } cs->exception_index = vector; env->error_code = event_inj_err; env->exception_is_int = 0; @@ -398,6 +401,9 @@ void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend) qemu_log_mask(CPU_LOG_TB_IN_ASM, "SOFT"); cpu_loop_exit(cs); break; + default: + cpu_vmexit(env, SVM_EXIT_ERR, 0, GETPC()); + break; } qemu_log_mask(CPU_LOG_TB_IN_ASM, " %#x %#x\n", cs->exception_index, env->error_code); From f594bfb79f572b27404d251f9758a36b83271580 Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Thu, 29 Jul 2021 09:56:38 +0200 Subject: [PATCH 5/7] target/i386: fix typo in ctl_has_irq The shift constant was incorrect, causing int_prio to always be zero. Signed-off-by: Lara Lazier [Rewritten commit message since v1 had already been included. - Paolo] Signed-off-by: Paolo Bonzini --- target/i386/tcg/sysemu/svm_helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/i386/tcg/sysemu/svm_helper.c b/target/i386/tcg/sysemu/svm_helper.c index 90a9de30f8..e151104b4e 100644 --- a/target/i386/tcg/sysemu/svm_helper.c +++ b/target/i386/tcg/sysemu/svm_helper.c @@ -70,7 +70,7 @@ static inline bool ctl_has_irq(uint32_t int_ctl) uint32_t int_prio; uint32_t tpr; - int_prio = (int_ctl & V_INTR_PRIO_MASK) >> V_INTR_MASKING_SHIFT; + int_prio = (int_ctl & V_INTR_PRIO_MASK) >> V_INTR_PRIO_SHIFT; tpr = int_ctl & V_TPR_MASK; return (int_ctl & V_IRQ_MASK) && (int_prio >= tpr); } From 3f55f97b14086b0f9f638e5bb784b3485b36d583 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 5 May 2021 19:13:12 +0400 Subject: [PATCH 6/7] meson: fix meson 0.58 warning with libvhost-user subproject MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Meson now checks that subprojects do not access files from parent project. While we all agree this is best practice, libvhost-user also want to share a few headers with QEMU, and libvhost-user isn't really a standalone project at this point (although this is making the dependency a bit more explicit). Signed-off-by: Marc-André Lureau Message-Id: <20210505151313.203258-1-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini --- subprojects/libvhost-user/include/atomic.h | 1 + subprojects/libvhost-user/libvhost-user.c | 2 +- subprojects/libvhost-user/meson.build | 6 +----- subprojects/libvhost-user/standard-headers/linux | 1 + 4 files changed, 4 insertions(+), 6 deletions(-) create mode 120000 subprojects/libvhost-user/include/atomic.h create mode 120000 subprojects/libvhost-user/standard-headers/linux diff --git a/subprojects/libvhost-user/include/atomic.h b/subprojects/libvhost-user/include/atomic.h new file mode 120000 index 0000000000..8c2be64f7b --- /dev/null +++ b/subprojects/libvhost-user/include/atomic.h @@ -0,0 +1 @@ +../../../include/qemu/atomic.h \ No newline at end of file diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c index fab7ca17ee..2971ba0112 100644 --- a/subprojects/libvhost-user/libvhost-user.c +++ b/subprojects/libvhost-user/libvhost-user.c @@ -40,7 +40,7 @@ #endif -#include "qemu/atomic.h" +#include "include/atomic.h" #include "libvhost-user.h" diff --git a/subprojects/libvhost-user/meson.build b/subprojects/libvhost-user/meson.build index b03446e7cd..39825d9404 100644 --- a/subprojects/libvhost-user/meson.build +++ b/subprojects/libvhost-user/meson.build @@ -4,21 +4,17 @@ project('libvhost-user', 'c', threads = dependency('threads') glib = dependency('glib-2.0') -inc = include_directories('../../include', '../../linux-headers') vhost_user = static_library('vhost-user', files('libvhost-user.c'), - include_directories: inc, dependencies: threads, c_args: '-D_GNU_SOURCE') executable('link-test', files('link-test.c'), - link_whole: vhost_user, - include_directories: inc) + link_whole: vhost_user) vhost_user_glib = static_library('vhost-user-glib', files('libvhost-user-glib.c'), - include_directories: inc, link_with: vhost_user, dependencies: glib) diff --git a/subprojects/libvhost-user/standard-headers/linux b/subprojects/libvhost-user/standard-headers/linux new file mode 120000 index 0000000000..15a2378139 --- /dev/null +++ b/subprojects/libvhost-user/standard-headers/linux @@ -0,0 +1 @@ +../../../include/standard-headers/linux \ No newline at end of file From 4fe29344bef6c54a6eff7aa0343754f8a9df5715 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= Date: Wed, 5 May 2021 19:13:13 +0400 Subject: [PATCH 7/7] libvhost-user: fix -Werror=format= warnings with __u64 fields MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ../subprojects/libvhost-user/libvhost-user.c:1070:12: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 3 has type ‘__u64’ {aka ‘long long unsigned int’} [-Werror=format=] 1070 | DPRINT(" desc_user_addr: 0x%016" PRIx64 "\n", vra->desc_user_addr); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~~~~ | | | __u64 {aka long long unsigned int} Rather than using %llx, which may fail if __u64 is declared differently elsewhere, let's just cast the values. Feel free to propose a better solution! Signed-off-by: Marc-André Lureau Message-Id: <20210505151313.203258-2-marcandre.lureau@redhat.com> Signed-off-by: Paolo Bonzini --- subprojects/libvhost-user/libvhost-user.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/subprojects/libvhost-user/libvhost-user.c b/subprojects/libvhost-user/libvhost-user.c index 2971ba0112..bf09693255 100644 --- a/subprojects/libvhost-user/libvhost-user.c +++ b/subprojects/libvhost-user/libvhost-user.c @@ -1067,10 +1067,10 @@ vu_set_vring_addr_exec(VuDev *dev, VhostUserMsg *vmsg) DPRINT("vhost_vring_addr:\n"); DPRINT(" index: %d\n", vra->index); DPRINT(" flags: %d\n", vra->flags); - DPRINT(" desc_user_addr: 0x%016" PRIx64 "\n", vra->desc_user_addr); - DPRINT(" used_user_addr: 0x%016" PRIx64 "\n", vra->used_user_addr); - DPRINT(" avail_user_addr: 0x%016" PRIx64 "\n", vra->avail_user_addr); - DPRINT(" log_guest_addr: 0x%016" PRIx64 "\n", vra->log_guest_addr); + DPRINT(" desc_user_addr: 0x%016" PRIx64 "\n", (uint64_t)vra->desc_user_addr); + DPRINT(" used_user_addr: 0x%016" PRIx64 "\n", (uint64_t)vra->used_user_addr); + DPRINT(" avail_user_addr: 0x%016" PRIx64 "\n", (uint64_t)vra->avail_user_addr); + DPRINT(" log_guest_addr: 0x%016" PRIx64 "\n", (uint64_t)vra->log_guest_addr); vq->vra = *vra; vq->vring.flags = vra->flags;