Skip to content

Commit

Permalink
linux-user: Fix qemu brk() to not zero bytes on current page
Browse files Browse the repository at this point in the history
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/write bytes above their
current heap address. On a phyiscal machine this does not trigger a
runtime error as long as the access happens on the same page. Additionally
the Linux kernel allocates only full pages and does no zeroing on already
allocated pages, even if the brk address is lowered.

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 page size is bigger
than the guest page size.

Signed-off-by: Helge Deller <deller@gmx.de>
Tested-by: "Markus F.X.J. Oberhumer" <markus@oberhumer.com>
Fixes: 86f0473 ("linux-user: Fix brk() to release pages")
Cc: qemu-stable@nongnu.org
Buglink: upx/upx#683
(cherry picked from commit 15ad985)
Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
  • Loading branch information
hdeller authored and Michael Tokarev committed Jul 31, 2023
1 parent 5de88d6 commit 0102c92
Showing 1 changed file with 4 additions and 6 deletions.
10 changes: 4 additions & 6 deletions linux-user/syscall.c
Original file line number Diff line number Diff line change
Expand Up @@ -829,18 +829,16 @@ 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;
}

/* 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);
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 0102c92

Please sign in to comment.