From cfae0537049d10e84a01cf9ce3e2d3833fd8e92d Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Mon, 15 Jan 2024 08:11:46 +0100 Subject: [PATCH 1/5] tests/qtest/meson.build: Bump the boot-serial-test timeout to 4 minutes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When running with TCI, the boot-serial-test can take longer than 3 minutes: https://gitlab.com/qemu-project/qemu/-/jobs/5890481086#L4774 Bump the timeout to 4 minutes to avoid CI failures here. Message-ID: <20240115071146.31213-1-thuth@redhat.com> Reviewed-by: "Daniel P. Berrangé" Signed-off-by: Thomas Huth --- tests/qtest/meson.build | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index fd40136fa9..78c298ce97 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -8,7 +8,7 @@ slow_qtests = { 'test-hmp' : 240, 'pxe-test': 600, 'prom-env-test': 360, - 'boot-serial-test': 180, + 'boot-serial-test': 240, 'qos-test': 120, } From d9e2da0ccc5b636afc632ba9cb95d83885e2d8dc Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Mon, 15 Jan 2024 08:02:23 +0100 Subject: [PATCH 2/5] tests/qtest/npcm7xx_watchdog_timer: Only test the corner cases by default MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The test_prescaler() part in the npcm7xx_watchdog_timer test is quite repetitive, testing all possible combinations of the WTCLK and WTIS bitfields. Since each test spins up a new instance of QEMU, this is rather an expensive test, especially on loaded host systems. For the normal quick test mode, it should be sufficient to test the corner settings of these fields (i.e. 0 and 3), so we can speed up this test in the default mode quite a bit. Message-ID: <20240115070223.30178-1-thuth@redhat.com> Reviewed-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- tests/qtest/npcm7xx_watchdog_timer-test.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/qtest/npcm7xx_watchdog_timer-test.c b/tests/qtest/npcm7xx_watchdog_timer-test.c index 4773a673b2..981b853c99 100644 --- a/tests/qtest/npcm7xx_watchdog_timer-test.c +++ b/tests/qtest/npcm7xx_watchdog_timer-test.c @@ -172,9 +172,10 @@ static void test_reset_action(gconstpointer watchdog) static void test_prescaler(gconstpointer watchdog) { const Watchdog *wd = watchdog; + int inc = g_test_quick() ? 3 : 1; - for (int wtclk = 0; wtclk < 4; ++wtclk) { - for (int wtis = 0; wtis < 4; ++wtis) { + for (int wtclk = 0; wtclk < 4; wtclk += inc) { + for (int wtis = 0; wtis < 4; wtis += inc) { QTestState *qts = qtest_init("-machine quanta-gsj"); qtest_irq_intercept_in(qts, "/machine/soc/a9mpcore/gic"); From 2d1f10229adde680234b8c975a4306b697c3b505 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 12 Jan 2024 17:47:17 +0100 Subject: [PATCH 3/5] qtest: Bump npcm7xx_watchdog_timer-test timeout to 2 minutes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The npcm7xx_watchdog_timer-test can take more than 60 seconds in SPEED=slow mode on a loaded host system. Bumping to 2 minutes will give more headroom. Message-ID: <20240112164717.1063954-1-thuth@redhat.com> Reviewed-by: "Daniel P. Berrangé" Signed-off-by: Thomas Huth --- tests/qtest/meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build index 78c298ce97..4293f3b133 100644 --- a/tests/qtest/meson.build +++ b/tests/qtest/meson.build @@ -4,6 +4,7 @@ slow_qtests = { 'device-introspect-test' : 720, 'migration-test' : 480, 'npcm7xx_pwm-test': 300, + 'npcm7xx_watchdog_timer-test': 120, 'qom-test' : 900, 'test-hmp' : 240, 'pxe-test': 600, From 043eaa0f0c8618382b2dba2f2e4fe762215b2e29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Wed, 3 Jan 2024 12:34:13 +0000 Subject: [PATCH 4/5] meson: mitigate against ROP exploits with -fzero-call-used-regs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To quote wikipedia: "Return-oriented programming (ROP) is a computer security exploit technique that allows an attacker to execute code in the presence of security defenses such as executable space protection and code signing. In this technique, an attacker gains control of the call stack to hijack program control flow and then executes carefully chosen machine instruction sequences that are already present in the machine's memory, called "gadgets". Each gadget typically ends in a return instruction and is located in a subroutine within the existing program and/or shared library code. Chained together, these gadgets allow an attacker to perform arbitrary operations on a machine employing defenses that thwart simpler attacks." QEMU is by no means perfect with an ever growing set of CVEs from flawed hardware device emulation, which could potentially be exploited using ROP techniques. Since GCC 11 there has been a compiler option that can mitigate against this exploit technique: -fzero-call-user-regs To understand it refer to these two resources: https://www.jerkeby.se/newsletter/posts/rop-reduction-zero-call-user-regs/ https://gcc.gnu.org/pipermail/gcc-patches/2020-August/552262.html I used two programs to scan qemu-system-x86_64 for ROP gadgets: https://github.com/0vercl0k/rp https://github.com/JonathanSalwan/ROPgadget When asked to find 8 byte gadgets, the 'rp' tool reports: A total of 440278 gadgets found. You decided to keep only the unique ones, 156143 unique gadgets found. While the ROPgadget tool reports: Unique gadgets found: 353122 With the --ropchain argument, the latter attempts to use the found gadgets to product a chain that can execute arbitrary syscalls. With current QEMU it succeeds in this task, which is an undesirable situation. With QEMU modified to use -fzero-call-user-regs=used-gpr the 'rp' tool reports A total of 528991 gadgets found. You decided to keep only the unique ones, 121128 unique gadgets found. This is 22% fewer unique gadgets While the ROPgadget tool reports: Unique gadgets found: 328605 This is 7% fewer unique gadgets. Crucially though, despite this more modest reduction, the ROPgadget tool is no longer able to identify a chain of gadgets for executing arbitrary syscalls. It fails at the very first step, unable to find gadgets for populating registers for a future syscall. Having said that, more advanced tools do still manage to put together a viable ROP chain. Also this only takes into account QEMU code. QEMU links to many 3rd party shared libraries and ideally all of them would be compiled with this same hardening. That becomes a distro policy question though. In terms of performance impact, TCG was used as an evaluation test case. We're not interested in protecting TCG since it isn't designed to provide a security barrier, but it is performance sensitive code, so useful as a guide to how other areas of QEMU might be impacted. With the -fzero-call-user-regs=used-gpr argument present, using the real world test of booting a linux kernel and having init immediately poweroff, there is a ~1% slow down in performance under TCG. The QEMU binary size also grows by approximately 1%. By comparison, using the more aggressive -fzero-call-user-regs=all, results in a slowdown of over 25% in TCG, which is clearly not an acceptable impact, and a binary size increase of 5%. Considering that 'used-gpr' successfully stopped ROPgadget assembling a chain, this more targeted protection is a justifiable hardening / performance tradeoff. Reviewed-by: Thomas Huth Signed-off-by: "Daniel P. Berrangé" Message-ID: <20240103123414.2401208-2-berrange@redhat.com> Signed-off-by: Thomas Huth --- meson.build | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/meson.build b/meson.build index 38deb9363c..1bda391de6 100644 --- a/meson.build +++ b/meson.build @@ -552,6 +552,17 @@ if get_option('cfi') add_global_link_arguments(cfi_flags, native: false, language: all_languages) endif +# Check further flags that make QEMU more robust against malicious parties + +hardening_flags = [ + # Zero out registers used during a function call + # upon its return. This makes it harder to assemble + # ROP gadgets into something usable + '-fzero-call-used-regs=used-gpr', +] + +qemu_common_flags += cc.get_supported_arguments(hardening_flags) + add_global_arguments(qemu_common_flags, native: false, language: all_languages) add_global_link_arguments(qemu_ldflags, native: false, language: all_languages) From 7ff9ff039380008952c6fd32011dd2a4d5666906 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Wed, 3 Jan 2024 12:34:14 +0000 Subject: [PATCH 5/5] meson: mitigate against use of uninitialize stack for exploits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When variables are used without being initialized, there is potential to take advantage of data that was pre-existing on the stack from an earlier call, to drive an exploit. It is good practice to always initialize variables, and the compiler can warn about flaws when -Wuninitialized is present. This warning, however, is by no means foolproof with its output varying depending on compiler version and which optimizations are enabled. The -ftrivial-auto-var-init option can be used to tell the compiler to always initialize all variables. This increases the security and predictability of the program, closing off certain attack vectors, reducing the risk of unsafe memory disclosure. While the option takes several possible values, using 'zero' is considered to be the option that is likely to lead to semantically correct or safe behaviour[1]. eg sizes/indexes are not likely to lead to out-of-bounds accesses when initialized to zero. Pointers are less likely to point something useful if initialized to zero. Even with -ftrivial-auto-var-init=zero set, GCC will still issue warnings with -Wuninitialized if it discovers a problem, so we are not loosing diagnostics for developers, just hardening runtime behaviour and making QEMU behave more predictably in case of hitting bad codepaths. [1] https://lists.llvm.org/pipermail/cfe-dev/2020-April/065221.html Signed-off-by: "Daniel P. Berrangé" Message-ID: <20240103123414.2401208-3-berrange@redhat.com> Signed-off-by: Thomas Huth --- meson.build | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/meson.build b/meson.build index 1bda391de6..d0329966f1 100644 --- a/meson.build +++ b/meson.build @@ -559,6 +559,11 @@ hardening_flags = [ # upon its return. This makes it harder to assemble # ROP gadgets into something usable '-fzero-call-used-regs=used-gpr', + + # Initialize all stack variables to zero. This makes + # it harder to take advantage of uninitialized stack + # data to drive exploits + '-ftrivial-auto-var-init=zero', ] qemu_common_flags += cc.get_supported_arguments(hardening_flags)