From 0f0a9e4e5c38c45ca7e6ac09cb36db21d42ec6dd Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 25 Nov 2022 09:30:54 +0100 Subject: [PATCH 1/3] tests/qtest/migration-test: Fix unlink error and memory leaks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When running the migration test compiled with Clang from Fedora 37 and sanitizers enabled, there is an error complaining about unlink(): ../tests/qtest/migration-test.c:1072:12: runtime error: null pointer passed as argument 1, which is declared to never be null /usr/include/unistd.h:858:48: note: nonnull attribute specified here SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../tests/qtest/migration-test.c:1072:12 in (test program exited with status code 1) TAP parsing error: Too few tests run (expected 33, got 20) The data->clientcert and data->clientkey pointers can indeed be unset in some tests, so we have to check them before calling unlink() with those. While we're at it, I also noticed that the code is only freeing some but not all of the allocated strings in this function, and indeed, valgrind is also complaining about memory leaks here. So let's call g_free() on all allocated strings to avoid leaking memory here. Message-Id: <20221125083054.117504-1-thuth@redhat.com> Tested-by: Bin Meng Reviewed-by: Daniel P. Berrangé Reviewed-by: Juan Quintela Signed-off-by: Thomas Huth --- tests/qtest/migration-test.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index 442998d9eb..dbde726adf 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -1066,15 +1066,27 @@ test_migrate_tls_x509_finish(QTestState *from, TestMigrateTLSX509Data *data = opaque; test_tls_cleanup(data->keyfile); - unlink(data->cacert); - unlink(data->servercert); - unlink(data->serverkey); - unlink(data->clientcert); - unlink(data->clientkey); - rmdir(data->workdir); - - g_free(data->workdir); g_free(data->keyfile); + + unlink(data->cacert); + g_free(data->cacert); + unlink(data->servercert); + g_free(data->servercert); + unlink(data->serverkey); + g_free(data->serverkey); + + if (data->clientcert) { + unlink(data->clientcert); + g_free(data->clientcert); + } + if (data->clientkey) { + unlink(data->clientkey); + g_free(data->clientkey); + } + + rmdir(data->workdir); + g_free(data->workdir); + g_free(data); } #endif /* CONFIG_TASN1 */ From 21be74a9a59d1e4954ebb59dcbee0fda0b19de00 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Thu, 1 Dec 2022 19:44:43 +0100 Subject: [PATCH 2/3] target/s390x/tcg: Fix and improve the SACF instruction The SET ADDRESS SPACE CONTROL FAST instruction is not privileged, it can be used from problem space, too. Just the switching to the home address space is privileged and should still generate a privilege exception. This bug is e.g. causing programs like Java that use the "getcpu" vdso kernel function to crash (see https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=990417#26 ). While we're at it, also check if DAT is not enabled. In that case the instruction is supposed to generate a special operation exception. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/655 Message-Id: <20221201184443.136355-1-thuth@redhat.com> Reviewed-by: Ilya Leoshkevich Reviewed-by: David Hildenbrand Reviewed-by: Richard Henderson Signed-off-by: Thomas Huth --- target/s390x/tcg/cc_helper.c | 7 +++++++ target/s390x/tcg/insn-data.h.inc | 2 +- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/target/s390x/tcg/cc_helper.c b/target/s390x/tcg/cc_helper.c index b2e8d3d9f5..b36f8cdc8b 100644 --- a/target/s390x/tcg/cc_helper.c +++ b/target/s390x/tcg/cc_helper.c @@ -487,6 +487,10 @@ void HELPER(sacf)(CPUS390XState *env, uint64_t a1) { HELPER_LOG("%s: %16" PRIx64 "\n", __func__, a1); + if (!(env->psw.mask & PSW_MASK_DAT)) { + tcg_s390_program_interrupt(env, PGM_SPECIAL_OP, GETPC()); + } + switch (a1 & 0xf00) { case 0x000: env->psw.mask &= ~PSW_MASK_ASC; @@ -497,6 +501,9 @@ void HELPER(sacf)(CPUS390XState *env, uint64_t a1) env->psw.mask |= PSW_ASC_SECONDARY; break; case 0x300: + if ((env->psw.mask & PSW_MASK_PSTATE) != 0) { + tcg_s390_program_interrupt(env, PGM_PRIVILEGED, GETPC()); + } env->psw.mask &= ~PSW_MASK_ASC; env->psw.mask |= PSW_ASC_HOME; break; diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc index 7e952bdfc8..54d4250c9f 100644 --- a/target/s390x/tcg/insn-data.h.inc +++ b/target/s390x/tcg/insn-data.h.inc @@ -1365,7 +1365,7 @@ /* SERVICE CALL LOGICAL PROCESSOR (PV hypercall) */ F(0xb220, SERVC, RRE, Z, r1_o, r2_o, 0, 0, servc, 0, IF_PRIV | IF_IO) /* SET ADDRESS SPACE CONTROL FAST */ - F(0xb279, SACF, S, Z, 0, a2, 0, 0, sacf, 0, IF_PRIV) + C(0xb279, SACF, S, Z, 0, a2, 0, 0, sacf, 0) /* SET CLOCK */ F(0xb204, SCK, S, Z, 0, m2_64a, 0, 0, sck, 0, IF_PRIV | IF_IO) /* SET CLOCK COMPARATOR */ From c1966f515d9bb6d8ed7076f4bebdc45407700100 Mon Sep 17 00:00:00 2001 From: Evgeny Ermakov Date: Sat, 26 Nov 2022 03:08:49 +1100 Subject: [PATCH 3/3] hw/display/next-fb: Fix comment typo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Evgeny Ermakov Message-Id: <20221125160849.23711-1-evgeny.v.ermakov@gmail.com> Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Peter Maydell Signed-off-by: Thomas Huth --- hw/display/next-fb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/display/next-fb.c b/hw/display/next-fb.c index dd6a1aa8ae..8446ff3c00 100644 --- a/hw/display/next-fb.c +++ b/hw/display/next-fb.c @@ -126,7 +126,7 @@ static void nextfb_class_init(ObjectClass *oc, void *data) set_bit(DEVICE_CATEGORY_DISPLAY, dc->categories); dc->realize = nextfb_realize; - /* Note: This device does not any state that we have to reset or migrate */ + /* Note: This device does not have any state that we have to reset or migrate */ } static const TypeInfo nextfb_info = {