[2/6] linux-user: Fix qemu brk() to not zero bytes on current page

Commit Message

Helge Deller July 17, 2023, 9:35 p.m. UTC

The qemu brk() implementation is too aggressive and cleans remaining bytes
on the current page above the last brk address.
But some existing applications are buggy and read or write to bytes above
their current heap address. On a phyiscal machine this does not trigger
any runtime errors (as long as the accesses happen on the same page only)
since the Linux kernel allocates only full pages and does no zeroing on
already allocated pages.

So, fix qemu to behave the same way as the kernel does. Do not touch
already allocated pages, and - when running with different page sizes of
guest and host - zero out only those memory areas where the host have a
bigger page size than the guest.

Signed-off-by: Helge Deller <[e-mail]>
Tested-by: Markus F.X.J. Oberhumer <[e-mail]>
Fixes: 86f04735ac ("linux-user: Fix brk() to release pages")
Buglink: https://github.com/upx/upx/issues/683
---
 linux-user/syscall.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

--
2.41.0

Patch

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index b9527ab00f..f877156ed3 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -829,10 +829,8 @@  abi_long do_brk(abi_ulong brk_val)

     /* brk_val and old target_brk might be on the same page */
     if (new_brk == TARGET_PAGE_ALIGN(target_brk)) {
-        if (brk_val > target_brk) {
-            /* empty remaining bytes in (possibly larger) host page */
-            memset(g2h_untagged(target_brk), 0, new_host_brk_page - target_brk);
-        }
+        /* empty remaining bytes in (possibly larger) host page */
+        memset(g2h_untagged(new_brk), 0, new_host_brk_page - new_brk);
         target_brk = brk_val;
         return target_brk;
     }
@@ -840,7 +838,7 @@  abi_long do_brk(abi_ulong brk_val)
     /* Release heap if necesary */
     if (new_brk < target_brk) {
         /* empty remaining bytes in (possibly larger) host page */
-        memset(g2h_untagged(brk_val), 0, new_host_brk_page - brk_val);
+        memset(g2h_untagged(new_brk), 0, new_host_brk_page - new_brk);

         /* free unused host pages and set new brk_page */
         target_munmap(new_host_brk_page, brk_page - new_host_brk_page);
@@ -873,7 +871,7 @@  abi_long do_brk(abi_ulong brk_val)
          * come from the remaining part of the previous page: it may
          * contains garbage data due to a previous heap usage (grown
          * then shrunken).  */
-        memset(g2h_untagged(target_brk), 0, brk_page - target_brk);
+        memset(g2h_untagged(brk_page), 0, HOST_PAGE_ALIGN(brk_page) - brk_page);

         target_brk = brk_val;
         brk_page = new_host_brk_page;