[PULL 0/5] qtest timeouts and ROP mitigation

[PULL 0/5] qtest timeouts and ROP mitigation

[Thomas Huth]

 Hi Peter!

The following changes since commit 977542ded7e6b28d2bc077bcda24568c716e393c:

  Merge tag 'pull-testing-updates-120124-2' of https://gitlab.com/stsquad/qemu into staging (2024-01-12 14:02:53 +0000)

are available in the Git repository at:

  https://gitlab.com/thuth/qemu.git tags/pull-request-2024-01-16

for you to fetch changes up to 7ff9ff039380008952c6fd32011dd2a4d5666906:

  meson: mitigate against use of uninitialize stack for exploits (2024-01-16 07:25:27 +0100)

----------------------------------------------------------------
* Improve the timeouts for some problematic qtests
* Enable some ROP mitigation compiler switches

----------------------------------------------------------------
Daniel P. Berrangé (2):
      meson: mitigate against ROP exploits with -fzero-call-used-regs
      meson: mitigate against use of uninitialize stack for exploits

Thomas Huth (3):
      tests/qtest/meson.build: Bump the boot-serial-test timeout to 4 minutes
      tests/qtest/npcm7xx_watchdog_timer: Only test the corner cases by default
      qtest: Bump npcm7xx_watchdog_timer-test timeout to 2 minutes

 meson.build                               | 16 ++++++++++++++++
 tests/qtest/npcm7xx_watchdog_timer-test.c |  5 +++--
 tests/qtest/meson.build                   |  3 ++-
 3 files changed, 21 insertions(+), 3 deletions(-)

[PULL 1/5] tests/qtest/meson.build: Bump the boot-serial-test timeout to 4 minutes

[Thomas Huth]

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é" <berrange@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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,
 }
 
-- 
2.43.0

[PULL 2/5] tests/qtest/npcm7xx_watchdog_timer: Only test the corner cases by default

[Thomas Huth]

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 <alex.bennee@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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");
-- 
2.43.0

[PULL 3/5] qtest: Bump npcm7xx_watchdog_timer-test timeout to 2 minutes

[Thomas Huth]

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é" <berrange@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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,
-- 
2.43.0

[PULL 4/5] meson: mitigate against ROP exploits with -fzero-call-used-regs

[Thomas Huth]

From: Daniel P. Berrangé <berrange@redhat.com>

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 <thuth@redhat.com>
Signed-off-by: "Daniel P. Berrangé" <berrange@redhat.com>
Message-ID: <20240103123414.2401208-2-berrange@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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)
 
-- 
2.43.0

[PULL 5/5] meson: mitigate against use of uninitialize stack for exploits

[Thomas Huth]

From: Daniel P. Berrangé <berrange@redhat.com>

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é" <berrange@redhat.com>
Message-ID: <20240103123414.2401208-3-berrange@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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)
-- 
2.43.0

Re: [PULL 0/5] qtest timeouts and ROP mitigation

[Peter Maydell]

On Tue, 16 Jan 2024 at 07:57, Thomas Huth <thuth@redhat.com> wrote:
>
>  Hi Peter!
>
> The following changes since commit 977542ded7e6b28d2bc077bcda24568c716e393c:
>
>   Merge tag 'pull-testing-updates-120124-2' of https://gitlab.com/stsquad/qemu into staging (2024-01-12 14:02:53 +0000)
>
> are available in the Git repository at:
>
>   https://gitlab.com/thuth/qemu.git tags/pull-request-2024-01-16
>
> for you to fetch changes up to 7ff9ff039380008952c6fd32011dd2a4d5666906:
>
>   meson: mitigate against use of uninitialize stack for exploits (2024-01-16 07:25:27 +0100)
>
> ----------------------------------------------------------------
> * Improve the timeouts for some problematic qtests
> * Enable some ROP mitigation compiler switches
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/9.0
for any user-visible changes.

-- PMM