Skip to content

Commit

Permalink
Merge tag 'linux-user-brk-fixes-pull-request' of https://github.com/h…
Browse files Browse the repository at this point in the history
…deller/qemu-hppa into staging

linux-user: brk() syscall fixes and armhf static binary fix

Commit 86f0473 ("linux-user: Fix brk() to release pages") introduced
the possibility for userspace applications to reduce memory footprint by
calling brk() with a lower address and as such free up memory, the same
way as the Linux kernel allows on physical machines.

This change introduced some failures for applications with errors like
- accesing bytes above the brk heap address on the same page,
- freeing memory below the initial brk address,
and introduced a behaviour which isn't done by the kernel (e.g. zeroing
memory above brk).

This patch series fixes those issues and has been tested with existing
programs (e.g. upx).

Additionally one patch fixes running static armhf executables (e.g. fstype)
which was broken since qemu-8.0.

Changes in v2:
- dropped patch to revert d28b3c9 ("linux-user: Make sure initial brk(0)
  is page-aligned")
- rephrased some commit messages
- fixed Cc email addresses, added new ones
- added R-b tags

Helge

# -----BEGIN PGP SIGNATURE-----
#
# iHUEABYKAB0WIQS86RI+GtKfB8BJu973ErUQojoPXwUCZLgGswAKCRD3ErUQojoP
# XwkUAQCKb/lkI3IYxiqO48rVyHtLPtkXd+WttFpeZ076p73LTgD+IEpHZL4WV1Rw
# 4+eqW9vswjZwp1xm9bItLdnP2hkyUgI=
# =K3Va
# -----END PGP SIGNATURE-----
# gpg: Signature made Wed 19 Jul 2023 16:52:19 BST
# gpg:                using EDDSA key BCE9123E1AD29F07C049BBDEF712B510A23A0F5F
# gpg: Good signature from "Helge Deller <deller@gmx.de>" [unknown]
# gpg:                 aka "Helge Deller <deller@kernel.org>" [unknown]
# gpg: WARNING: This key is not certified with a trusted signature!
# gpg:          There is no indication that the signature belongs to the owner.
# Primary key fingerprint: 4544 8228 2CD9 10DB EF3D  25F8 3E5F 3D04 A7A2 4603
#      Subkey fingerprint: BCE9 123E 1AD2 9F07 C049  BBDE F712 B510 A23A 0F5F

* tag 'linux-user-brk-fixes-pull-request' of https://github.com/hdeller/qemu-hppa:
  linux-user: Fix qemu-arm to run static armhf binaries
  linux-user: Fix strace output for old_mmap
  linux-user: Fix signed math overflow in brk() syscall
  linux-user: Prohibit brk() to to shrink below initial heap address
  linux-user: Fix qemu brk() to not zero bytes on current page

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
  • Loading branch information
pm215 committed Jul 20, 2023
2 parents 2c27fdc + 518f322 commit 67d1f0a
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 13 deletions.
7 changes: 7 additions & 0 deletions linux-user/elfload.c
Original file line number Diff line number Diff line change
Expand Up @@ -3618,6 +3618,13 @@ int load_elf_binary(struct linux_binprm *bprm, struct image_info *info)

if (elf_interpreter) {
load_elf_interp(elf_interpreter, &interp_info, bprm->buf);
/*
* adjust brk address if the interpreter was loaded above the main
* executable, e.g. happens with static binaries on armhf
*/
if (interp_info.brk > info->brk) {
info->brk = interp_info.brk;
}

/* If the program interpreter is one of these two, then assume
an iBCS2 image. Otherwise assume a native linux image. */
Expand Down
49 changes: 45 additions & 4 deletions linux-user/strace.c
Original file line number Diff line number Diff line change
Expand Up @@ -3767,10 +3767,24 @@ print_utimensat(CPUArchState *cpu_env, const struct syscallname *name,

#if defined(TARGET_NR_mmap) || defined(TARGET_NR_mmap2)
static void
print_mmap(CPUArchState *cpu_env, const struct syscallname *name,
print_mmap_both(CPUArchState *cpu_env, const struct syscallname *name,
abi_long arg0, abi_long arg1, abi_long arg2,
abi_long arg3, abi_long arg4, abi_long arg5)
{
abi_long arg3, abi_long arg4, abi_long arg5,
bool is_old_mmap)
{
if (is_old_mmap) {
abi_ulong *v;
abi_ulong argp = arg0;
if (!(v = lock_user(VERIFY_READ, argp, 6 * sizeof(abi_ulong), 1)))
return;
arg0 = tswapal(v[0]);
arg1 = tswapal(v[1]);
arg2 = tswapal(v[2]);
arg3 = tswapal(v[3]);
arg4 = tswapal(v[4]);
arg5 = tswapal(v[5]);
unlock_user(v, argp, 0);
}
print_syscall_prologue(name);
print_pointer(arg0, 0);
print_raw_param("%d", arg1, 0);
Expand All @@ -3780,7 +3794,34 @@ print_mmap(CPUArchState *cpu_env, const struct syscallname *name,
print_raw_param("%#x", arg5, 1);
print_syscall_epilogue(name);
}
#define print_mmap2 print_mmap
#endif

#if defined(TARGET_NR_mmap)
static void
print_mmap(CPUArchState *cpu_env, const struct syscallname *name,
abi_long arg0, abi_long arg1, abi_long arg2,
abi_long arg3, abi_long arg4, abi_long arg5)
{
return print_mmap_both(cpu_env, name, arg0, arg1, arg2, arg3,
arg4, arg5,
#if defined(TARGET_NR_mmap2)
true
#else
false
#endif
);
}
#endif

#if defined(TARGET_NR_mmap2)
static void
print_mmap2(CPUArchState *cpu_env, const struct syscallname *name,
abi_long arg0, abi_long arg1, abi_long arg2,
abi_long arg3, abi_long arg4, abi_long arg5)
{
return print_mmap_both(cpu_env, name, arg0, arg1, arg2, arg3,
arg4, arg5, false);
}
#endif

#ifdef TARGET_NR_mprotect
Expand Down
23 changes: 14 additions & 9 deletions linux-user/syscall.c
Original file line number Diff line number Diff line change
Expand Up @@ -801,12 +801,13 @@ static inline int host_to_target_sock_type(int host_type)
return target_type;
}

static abi_ulong target_brk;
static abi_ulong target_brk, initial_target_brk;
static abi_ulong brk_page;

void target_set_brk(abi_ulong new_brk)
{
target_brk = TARGET_PAGE_ALIGN(new_brk);
initial_target_brk = target_brk;
brk_page = HOST_PAGE_ALIGN(target_brk);
}

Expand All @@ -824,23 +825,26 @@ abi_long do_brk(abi_ulong brk_val)
return target_brk;
}

/* do not allow to shrink below initial brk value */
if (brk_val < initial_target_brk) {
brk_val = initial_target_brk;
}

new_brk = TARGET_PAGE_ALIGN(brk_val);
new_host_brk_page = HOST_PAGE_ALIGN(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 All @@ -856,12 +860,13 @@ abi_long do_brk(abi_ulong brk_val)
* itself); instead we treat "mapped but at wrong address" as
* a failure and unmap again.
*/
new_alloc_size = new_host_brk_page - brk_page;
if (new_alloc_size) {
if (new_host_brk_page > brk_page) {
new_alloc_size = new_host_brk_page - brk_page;
mapped_addr = get_errno(target_mmap(brk_page, new_alloc_size,
PROT_READ|PROT_WRITE,
MAP_ANON|MAP_PRIVATE, 0, 0));
} else {
new_alloc_size = 0;
mapped_addr = brk_page;
}

Expand All @@ -873,7 +878,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 67d1f0a

Please sign in to comment.