[PATCH qemu v2 0/2] Bugfix: TCG cross-page overflow for 32 bit guest

[PATCH qemu v2 0/2] Bugfix: TCG cross-page overflow for 32 bit guest

[~percival_foss]

Since v1:
-Updated git commits to have proper signed off by tags/messages

Our company found and fixed a bug in TCG that caused 32 bit guests
running on 64 bit hosts to crash. If a 32 bit host has a page
translation that crosses beyond the last page. This causes a invalid
translation to be added to the translation table causing a crash.

Since this is our first patch we tried following the wiki guidelines and
broke the patch into a fix and test. More info about each is given in
their respective commits. Any feedback is appreciated!

Signed off by: Percival Engineering <foss@percivaleng.com>

Percival Foss (2):
  accel/tcg: Fixed cross-page overflow for 32 bit guest
  tests/functional: Added cross page overflow test

 accel/tcg/cputlb.c                    |  7 +++
 tests/functional/meson.build          |  1 +
 tests/functional/test_ppc_pegasos2.py | 69 +++++++++++++++++++++++++++
 3 files changed, 77 insertions(+)
 create mode 100755 tests/functional/test_ppc_pegasos2.py

-- 
2.45.3

[PATCH qemu v2 1/2] accel/tcg: Fixed cross-page overflow for 32 bit guest

[~percival_foss]

From: Percival Foss <foss@percivaleng.com>

The bug being resolved is that the current code in mmu_lookup() assumes
a valid 64-bit address space. If a guest has a 32-bit address space, a
page translation that crosses beyond the last page in the address space
will overflow out of the allocated guest virtual memory space in the
QEMU application and cause it to crash. In this case the first page will
be the last of the 32-bit address space (for example 0xFFFFF000 for 4K
page sizes) and the second page will overflow to a page beyond the
32-bit address space (0x100000000 in the very same example). An invalid
translation will be added to the cpu translation table from the second
page. Thought the translation will be for page address 0x100000000,
checks in other parts of the codebase actually enforce using only 32
bits, and will match this translation. Part of the stored translation is
the effective address, and another part is the addend to be used to
offset into the QEMU process's virtual memory space. The addend will
incorporate the 0x100000000 and offset into likely invalid virtual
address space.

The fix in the diff checks if the target is 32 bits and wraps the second
page address to the beginning of the memory space.

Signed off by: Percival Engineering <foss@percivaleng.com>
---
 accel/tcg/cputlb.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index fb22048876..457b3f8ec7 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1767,6 +1767,13 @@ static bool mmu_lookup(CPUState *cpu, vaddr addr, MemOpIdx oi,
         l->page[1].size = l->page[0].size - size0;
         l->page[0].size = size0;
 
+        /* check for wrapping address space on page crossing if target is 32 bit */
+        #if TARGET_LONG_BITS == 32
+        if (l->page[1].addr >= (1UL << TARGET_LONG_BITS)) {
+            l->page[1].addr %= (1UL << TARGET_LONG_BITS);
+        }
+        # endif
+
         /*
          * Lookup both pages, recognizing exceptions from either.  If the
          * second lookup potentially resized, refresh first CPUTLBEntryFull.
-- 
2.45.3

Re: [PATCH qemu v2 1/2] accel/tcg: Fixed cross-page overflow for 32 bit guest

[Richard Henderson]

On 4/29/25 09:03, ~percival_foss wrote:
> From: Percival Foss <foss@percivaleng.com>
> 
> The bug being resolved is that the current code in mmu_lookup() assumes
> a valid 64-bit address space. If a guest has a 32-bit address space, a
> page translation that crosses beyond the last page in the address space
> will overflow out of the allocated guest virtual memory space in the
> QEMU application and cause it to crash. In this case the first page will
> be the last of the 32-bit address space (for example 0xFFFFF000 for 4K
> page sizes) and the second page will overflow to a page beyond the
> 32-bit address space (0x100000000 in the very same example). An invalid
> translation will be added to the cpu translation table from the second
> page. Thought the translation will be for page address 0x100000000,
> checks in other parts of the codebase actually enforce using only 32
> bits, and will match this translation. Part of the stored translation is
> the effective address, and another part is the addend to be used to
> offset into the QEMU process's virtual memory space. The addend will
> incorporate the 0x100000000 and offset into likely invalid virtual
> address space.
> 
> The fix in the diff checks if the target is 32 bits and wraps the second
> page address to the beginning of the memory space.
> 
> Signed off by: Percival Engineering <foss@percivaleng.com>
> ---
>   accel/tcg/cputlb.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
> index fb22048876..457b3f8ec7 100644
> --- a/accel/tcg/cputlb.c
> +++ b/accel/tcg/cputlb.c
> @@ -1767,6 +1767,13 @@ static bool mmu_lookup(CPUState *cpu, vaddr addr, MemOpIdx oi,
>           l->page[1].size = l->page[0].size - size0;
>           l->page[0].size = size0;
>   
> +        /* check for wrapping address space on page crossing if target is 32 bit */
> +        #if TARGET_LONG_BITS == 32
> +        if (l->page[1].addr >= (1UL << TARGET_LONG_BITS)) {
> +            l->page[1].addr %= (1UL << TARGET_LONG_BITS);
> +        }
> +        # endif

I agree something needs doing, but this isn't it.

This needs some sort of per-mmu mask, set when the cpu changes modes.  For instance, you 
test ppc32, but the same thing can happen with ppc64 when MSR[SF] is clear.

There are a fair number of other targets which can see the same issue, some more 
complicated than this.  For instance, s390x in 24-bit mode or RISC-V with the pointer 
masking extension.

This needs some sort of extension to CPUTLBDesc, and a new variation on 
tlb_flush_by_mmuidx in order for the target to clear the tlb while changing the mask 
whenever the target cpu changes modes.


r~

[PATCH qemu v2 2/2] tests/functional: Added cross page overflow test

[~percival_foss]

From: Percival Foss <foss@percivaleng.com>

This test utilizes pegasos2 to show the cross page overflow bug on 32 bit systems. We patch the firmware
image to cause the overflow. The instructions to do this are as follows:
li r3, 0
li r4, -1
lwz r5, 0x0(r4)
lwz r5, 0x0(r3)

This test will add an invalid translation when ran and then crash QEMU. If the fix is applied then pegasos2
will start properly and not crash.

Signed off by:Percival Engineering <foss@percivalemg.com>
---
 tests/functional/meson.build          |  1 +
 tests/functional/test_ppc_pegasos2.py | 69 +++++++++++++++++++++++++++
 2 files changed, 70 insertions(+)
 create mode 100755 tests/functional/test_ppc_pegasos2.py

diff --git a/tests/functional/meson.build b/tests/functional/meson.build
index 0f8be30fe2..6641b878c3 100644
--- a/tests/functional/meson.build
+++ b/tests/functional/meson.build
@@ -213,6 +213,7 @@ tests_ppc_system_thorough = [
   'ppc_bamboo',
   'ppc_mac',
   'ppc_mpc8544ds',
+  'ppc_pegasos2',
   'ppc_replay',
   'ppc_sam460ex',
   'ppc_tuxrun',
diff --git a/tests/functional/test_ppc_pegasos2.py b/tests/functional/test_ppc_pegasos2.py
new file mode 100755
index 0000000000..ef76745068
--- /dev/null
+++ b/tests/functional/test_ppc_pegasos2.py
@@ -0,0 +1,69 @@
+#!/usr/bin/env python3
+#
+# Test AmigaNG boards
+#
+# Copyright (c) 2023 BALATON Zoltan
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or
+# later.  See the COPYING file in the top-level directory.
+
+import subprocess
+
+from qemu_test import QemuSystemTest, Asset
+from qemu_test import wait_for_console_pattern
+from zipfile import ZipFile
+
+class Pegasos2Machine(QemuSystemTest):
+
+    timeout = 90
+
+    ASSET_IMAGE = Asset(
+        ('https://web.archive.org/web/20071021223056if_/http://www.bplan-gmbh.de/up050404/up050404'),
+        '0b4ff042b293033e094b47ac7051824fc45f83adb340d455a17db1674b0150b0c60ffc624ac766f5369cd79f0447214d468baa182c1f18c5e04cd23a50f0b9a2')
+
+    def test_ppc_pegasos2(self):
+        self.require_accelerator("tcg")
+        self.set_machine('pegasos2')
+        file_path = self.ASSET_IMAGE.fetch()
+        bios_fh = open(self.workdir + "/pegasos2.rom", "wb")
+        subprocess.run(['tail', '-c','+85581', file_path], stdout=bios_fh)
+        bios_fh.close()
+        subprocess.run(['truncate', '-s', '524288', self.workdir + "/pegasos2.rom"], )
+
+        self.vm.set_console()
+        self.vm.add_args('-bios', self.workdir + '/pegasos2.rom')
+        self.vm.launch()
+        wait_for_console_pattern(self, 'SmartFirmware:')
+
+    def test_ppc_pegasos2_test_tcg_crosspage_overflow_bug(self):
+        self.require_accelerator("tcg")
+        self.set_machine('pegasos2')
+        file_path = self.ASSET_IMAGE.fetch()
+        bios_fh = open(self.workdir + "/pegasos2.rom", "wb")
+        subprocess.run(['tail', '-c','+85581', file_path], stdout=bios_fh)
+        bios_fh.close()
+        subprocess.run(['truncate', '-s', '524288', self.workdir + "/pegasos2.rom"], )
+
+        with open(self.workdir + "/pegasos2.rom", "rb") as bios_fh:
+            bios_data = bios_fh.read()
+        
+        # Patch the firmware image with the following instructions that will cause tcg to crash for 32-bit guests on 64-bit platforms:
+        #   li r3, 0
+        #   li r4, -1
+        #   lwz r5, 0x0(r4)
+        #   lwz r5, 0x0(r3)
+
+        bios_data_new = bios_data[:0x6c10] + b'\x38\x60\x00\x00' + b'\x38\x80\xff\xff' + b'\x80\xa4\x00\x00' + b'\x80\xa3\x00\x00' + bios_data[0x6c20:]
+        with open(self.workdir + "/pegasos2_new.rom", "wb") as bios_new_fh:
+            bios_new_fh.write(bios_data_new)
+
+        self.vm.set_console()
+        self.vm.add_args('-bios', self.workdir + '/pegasos2_new.rom')
+        self.vm.launch()
+        wait_for_console_pattern(self, 'Releasing IDE reset')
+
+        # set $pc = 0 and expect crash
+
+
+if __name__ == '__main__':
+    QemuSystemTest.main()
-- 
2.45.3