* [PULL 1/5] linux-user: Fix qemu brk() to not zero bytes on current page
2023-07-19 15:52 [PULL 0/5] Linux user brk fixes patches Helge Deller
@ 2023-07-19 15:52 ` Helge Deller
2023-07-19 15:52 ` [PULL 2/5] linux-user: Prohibit brk() to to shrink below initial heap address Helge Deller
` (4 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Helge Deller @ 2023-07-19 15:52 UTC (permalink / raw)
To: Richard Henderson, Laurent Vivier, Andreas Schwab,
Michael Tokarev, qemu-devel
Cc: Helge Deller, Markus F.X.J. Oberhumer, qemu-stable
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: 86f04735ac ("linux-user: Fix brk() to release pages")
Cc: qemu-stable@nongnu.org
Buglink: https://github.com/upx/upx/issues/683
---
linux-user/syscall.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index c99ef9c01e..ee54eed33b 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;
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PULL 2/5] linux-user: Prohibit brk() to to shrink below initial heap address
2023-07-19 15:52 [PULL 0/5] Linux user brk fixes patches Helge Deller
2023-07-19 15:52 ` [PULL 1/5] linux-user: Fix qemu brk() to not zero bytes on current page Helge Deller
@ 2023-07-19 15:52 ` Helge Deller
2023-07-19 15:52 ` [PULL 3/5] linux-user: Fix signed math overflow in brk() syscall Helge Deller
` (3 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Helge Deller @ 2023-07-19 15:52 UTC (permalink / raw)
To: Richard Henderson, Laurent Vivier, Andreas Schwab,
Michael Tokarev, qemu-devel
Cc: Helge Deller, Markus F.X.J. Oberhumer, qemu-stable
Since commit 86f04735ac ("linux-user: Fix brk() to release pages") it's
possible for userspace applications to reduce their memory footprint by
calling brk() with a lower address and free up memory. Before that commit
guest heap memory was never unmapped.
But the Linux kernel prohibits to reduce brk() below the initial memory
address which is set at startup by the set_brk() function in binfmt_elf.c.
Such a range check was missed in commit 86f04735ac.
This patch adds the missing check by storing the initial brk value in
initial_target_brk and verify any new brk addresses against that value.
Tested with the i386 upx binary from
https://github.com/upx/upx/releases/download/v4.0.2/upx-4.0.2-i386_linux.tar.xz
Signed-off-by: Helge Deller <deller@gmx.de>
Tested-by: "Markus F.X.J. Oberhumer" <markus@oberhumer.com>
Fixes: 86f04735ac ("linux-user: Fix brk() to release pages")
Cc: qemu-stable@nongnu.org
Buglink: https://github.com/upx/upx/issues/683
---
linux-user/syscall.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index ee54eed33b..125fcbe423 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -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);
}
@@ -824,6 +825,11 @@ 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);
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PULL 3/5] linux-user: Fix signed math overflow in brk() syscall
2023-07-19 15:52 [PULL 0/5] Linux user brk fixes patches Helge Deller
2023-07-19 15:52 ` [PULL 1/5] linux-user: Fix qemu brk() to not zero bytes on current page Helge Deller
2023-07-19 15:52 ` [PULL 2/5] linux-user: Prohibit brk() to to shrink below initial heap address Helge Deller
@ 2023-07-19 15:52 ` Helge Deller
2023-07-19 15:52 ` [PULL 4/5] linux-user: Fix strace output for old_mmap Helge Deller
` (2 subsequent siblings)
5 siblings, 0 replies; 12+ messages in thread
From: Helge Deller @ 2023-07-19 15:52 UTC (permalink / raw)
To: Richard Henderson, Laurent Vivier, Andreas Schwab,
Michael Tokarev, qemu-devel
Cc: Helge Deller, Markus F.X.J. Oberhumer,
Philippe Mathieu-Daudé, qemu-stable
Fix the math overflow when calculating the new_malloc_size.
new_host_brk_page and brk_page are unsigned integers. If userspace
reduces the heap, new_host_brk_page is lower than brk_page which results
in a huge positive number (but should actually be negative).
Fix it by adding a proper check and as such make the code more readable.
Signed-off-by: Helge Deller <deller@gmx.de>
Tested-by: "Markus F.X.J. Oberhumer" <markus@oberhumer.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Fixes: 86f04735ac ("linux-user: Fix brk() to release pages")
Cc: qemu-stable@nongnu.org
Buglink: https://github.com/upx/upx/issues/683
---
linux-user/syscall.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 125fcbe423..95727a816a 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -860,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;
}
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PULL 4/5] linux-user: Fix strace output for old_mmap
2023-07-19 15:52 [PULL 0/5] Linux user brk fixes patches Helge Deller
` (2 preceding siblings ...)
2023-07-19 15:52 ` [PULL 3/5] linux-user: Fix signed math overflow in brk() syscall Helge Deller
@ 2023-07-19 15:52 ` Helge Deller
2023-07-19 15:52 ` [PULL 5/5] linux-user: Fix qemu-arm to run static armhf binaries Helge Deller
2023-07-21 9:51 ` [PULL 0/5] Linux user brk fixes patches Peter Maydell
5 siblings, 0 replies; 12+ messages in thread
From: Helge Deller @ 2023-07-19 15:52 UTC (permalink / raw)
To: Richard Henderson, Laurent Vivier, Andreas Schwab,
Michael Tokarev, qemu-devel
Cc: Helge Deller, John Reiser
The old_mmap syscall (e.g. on i386) hands over the parameters in
a struct. Adjust the strace output to print the correct values.
Signed-off-by: Helge Deller <deller@gmx.de>
Reported-by: John Reiser <jreiser@BitWagon.com>
Closes: https://gitlab.com/qemu-project/qemu/-/issues/1760
---
linux-user/strace.c | 49 +++++++++++++++++++++++++++++++++++++++++----
1 file changed, 45 insertions(+), 4 deletions(-)
diff --git a/linux-user/strace.c b/linux-user/strace.c
index bbd29148d4..e0ab8046ec 100644
--- a/linux-user/strace.c
+++ b/linux-user/strace.c
@@ -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);
@@ -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
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [PULL 5/5] linux-user: Fix qemu-arm to run static armhf binaries
2023-07-19 15:52 [PULL 0/5] Linux user brk fixes patches Helge Deller
` (3 preceding siblings ...)
2023-07-19 15:52 ` [PULL 4/5] linux-user: Fix strace output for old_mmap Helge Deller
@ 2023-07-19 15:52 ` Helge Deller
2023-07-21 15:14 ` Michael Tokarev
2023-07-21 9:51 ` [PULL 0/5] Linux user brk fixes patches Peter Maydell
5 siblings, 1 reply; 12+ messages in thread
From: Helge Deller @ 2023-07-19 15:52 UTC (permalink / raw)
To: Richard Henderson, Laurent Vivier, Andreas Schwab,
Michael Tokarev, qemu-devel
Cc: Helge Deller, qemu-stable
qemu-user crashes immediately when running static binaries on the armhf
architecture. The problem is the memory layout where the executable is
loaded before the interpreter library, in which case the reserved brk
region clashes with the interpreter code and is released before qemu
tries to start the program.
At load time qemu calculates a brk value for interpreter and executable
each. The fix is to choose the higher one of both.
Signed-off-by: Helge Deller <deller@gmx.de>
Cc: Andreas Schwab <schwab@suse.de>
Cc: qemu-stable@nongnu.org
Reported-by: Venkata.Pyla@toshiba-tsip.com
Closes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1040981
---
linux-user/elfload.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index a26200d9f3..94951630b1 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -3615,6 +3615,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. */
--
2.41.0
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PULL 5/5] linux-user: Fix qemu-arm to run static armhf binaries
2023-07-19 15:52 ` [PULL 5/5] linux-user: Fix qemu-arm to run static armhf binaries Helge Deller
@ 2023-07-21 15:14 ` Michael Tokarev
2023-07-21 15:20 ` Michael Tokarev
` (2 more replies)
0 siblings, 3 replies; 12+ messages in thread
From: Michael Tokarev @ 2023-07-21 15:14 UTC (permalink / raw)
To: Helge Deller, Richard Henderson, Laurent Vivier, Andreas Schwab,
qemu-devel, Peter Maydell
Cc: qemu-stable
19.07.2023 18:52, Helge Deller wrote:
> qemu-user crashes immediately when running static binaries on the armhf
> architecture. The problem is the memory layout where the executable is
> loaded before the interpreter library, in which case the reserved brk
> region clashes with the interpreter code and is released before qemu
> tries to start the program.
>
> At load time qemu calculates a brk value for interpreter and executable
> each. The fix is to choose the higher one of both.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
> Cc: Andreas Schwab <schwab@suse.de>
> Cc: qemu-stable@nongnu.org
> Reported-by: Venkata.Pyla@toshiba-tsip.com
> Closes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1040981
> ---
> linux-user/elfload.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index a26200d9f3..94951630b1 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -3615,6 +3615,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;
> + }
So, this is kinda amusing.
This broke arm64, ppc64el and s390x:
arm64$ ./qemu-aarch64 /bin/sh -c '/bin/ls -dCFl *[t]* >/dev/null'
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
Segmentation fault
(it was just a quick test from debian qemu-user package).
Reverting this patch makes it work again..
*Sigh*.
(This is currently broken in debian unstable too, - this is how I
noticed the breakage).
Thanks,
/mjt
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PULL 5/5] linux-user: Fix qemu-arm to run static armhf binaries
2023-07-21 15:14 ` Michael Tokarev
@ 2023-07-21 15:20 ` Michael Tokarev
2023-07-21 21:37 ` Helge Deller
2023-07-24 9:45 ` Helge Deller
2 siblings, 0 replies; 12+ messages in thread
From: Michael Tokarev @ 2023-07-21 15:20 UTC (permalink / raw)
To: Helge Deller, Richard Henderson, Laurent Vivier, Andreas Schwab,
qemu-devel, Peter Maydell
Cc: qemu-stable
21.07.2023 18:14, Michael Tokarev пишет:
> 19.07.2023 18:52, Helge Deller wrote:
>> qemu-user crashes immediately when running static binaries on the armhf
>> architecture. The problem is the memory layout where the executable is
>> loaded before the interpreter library, in which case the reserved brk
>> region clashes with the interpreter code and is released before qemu
>> tries to start the program.
>>
>> At load time qemu calculates a brk value for interpreter and executable
>> each. The fix is to choose the higher one of both.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> Cc: Andreas Schwab <schwab@suse.de>
>> Cc: qemu-stable@nongnu.org
>> Reported-by: Venkata.Pyla@toshiba-tsip.com
>> Closes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1040981
>> ---
>> linux-user/elfload.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index a26200d9f3..94951630b1 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -3615,6 +3615,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;
>> + }
I've added printf() inside this if() block, on arm64 it produces:
fixing brk: interp_info.brk=0x5502875358 info=>brk=0x5500032ec0
FWIW,
/mjt
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PULL 5/5] linux-user: Fix qemu-arm to run static armhf binaries
2023-07-21 15:14 ` Michael Tokarev
2023-07-21 15:20 ` Michael Tokarev
@ 2023-07-21 21:37 ` Helge Deller
2023-07-22 7:37 ` Michael Tokarev
2023-07-24 9:45 ` Helge Deller
2 siblings, 1 reply; 12+ messages in thread
From: Helge Deller @ 2023-07-21 21:37 UTC (permalink / raw)
To: Michael Tokarev, Richard Henderson, Laurent Vivier,
Andreas Schwab, qemu-devel, Peter Maydell
Cc: qemu-stable
On 7/21/23 17:14, Michael Tokarev wrote:
> 19.07.2023 18:52, Helge Deller wrote:
>> qemu-user crashes immediately when running static binaries on the armhf
>> architecture. The problem is the memory layout where the executable is
>> loaded before the interpreter library, in which case the reserved brk
>> region clashes with the interpreter code and is released before qemu
>> tries to start the program.
>>
>> At load time qemu calculates a brk value for interpreter and executable
>> each. The fix is to choose the higher one of both.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> Cc: Andreas Schwab <schwab@suse.de>
>> Cc: qemu-stable@nongnu.org
>> Reported-by: Venkata.Pyla@toshiba-tsip.com
>> Closes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1040981
>> ---
>> linux-user/elfload.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index a26200d9f3..94951630b1 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -3615,6 +3615,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;
>> + }
>
> So, this is kinda amusing.
> This broke arm64, ppc64el and s390x:
>
> arm64$ ./qemu-aarch64 /bin/sh -c '/bin/ls -dCFl *[t]* >/dev/null'
> qemu: uncaught target signal 11 (Segmentation fault) - core dumped
> Segmentation fault
>
> (it was just a quick test from debian qemu-user package).
>
> Reverting this patch makes it work again..
>
> *Sigh*.
Argh, that's really unfortunate.
I just tested myself.
Running static busybox binary did work for me:
# ./qemu-aarch64 busybox
BusyBox v1.30.1 (Debian 1:1.30.1-6+b3) multi-call binary.
BusyBox is copyrighted by many authors between 1998-2015.
....
I'd like to test dynamic binary as well, but I'm currently failing
to set up an aarch64 chroot here.
Sadly I won't have time to do any further testing until sunday evening
(travelling over the weekend).
Maybe someone else can try? I leave it up to Peter if he wants to revert
that patch right now, or if it can wait a few days until I'm back?
Helge
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PULL 5/5] linux-user: Fix qemu-arm to run static armhf binaries
2023-07-21 21:37 ` Helge Deller
@ 2023-07-22 7:37 ` Michael Tokarev
0 siblings, 0 replies; 12+ messages in thread
From: Michael Tokarev @ 2023-07-22 7:37 UTC (permalink / raw)
To: Helge Deller, Richard Henderson, Laurent Vivier, Andreas Schwab,
qemu-devel, Peter Maydell
Cc: qemu-stable
22.07.2023 00:37, Helge Deller wrote:
..
>> So, this is kinda amusing.
>> This broke arm64, ppc64el and s390x:
>>
>> arm64$ ./qemu-aarch64 /bin/sh -c '/bin/ls -dCFl *[t]* >/dev/null'
>> qemu: uncaught target signal 11 (Segmentation fault) - core dumped
>> Segmentation fault
Note: I run it on native arm64, so it is arm64 on arm64, -
this is a quick test I had from the debian qemu autopkgtest (which
run /bin/ls under qemu and natively and compares the result). I
haven't tried to reproduce it locally on amd64 host, - I did it on
a debian arm64 porterbox (which I was logged on anyway to debug a
different issue on armel, with qemu git tree already cloned).
> Argh, that's really unfortunate.
> I just tested myself.
> Running static busybox binary did work for me:
> # ./qemu-aarch64 busybox
It didn't trigger with ls, but it did when I run something from
emulated /bin/sh.
This whole email was more like a heads-up/warning, to collect more
details later, - and maybe someone knows what the problem is already
if it is obvious.
..
> Maybe someone else can try? I leave it up to Peter if he wants to revert
> that patch right now, or if it can wait a few days until I'm back?
For the time being, how about a quick-n-hacky band-aid, to include
this fixup only where the original prob actually triggered in the
first place?
Like, if the target is armel? Something like
#if defined(TARGET_ARM) && !defined(TARGET_AARCH64)
or what's the right preprocessor condition is?
Thanks,
/mjt
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PULL 5/5] linux-user: Fix qemu-arm to run static armhf binaries
2023-07-21 15:14 ` Michael Tokarev
2023-07-21 15:20 ` Michael Tokarev
2023-07-21 21:37 ` Helge Deller
@ 2023-07-24 9:45 ` Helge Deller
2 siblings, 0 replies; 12+ messages in thread
From: Helge Deller @ 2023-07-24 9:45 UTC (permalink / raw)
To: Michael Tokarev, Richard Henderson, Laurent Vivier,
Andreas Schwab, qemu-devel, Peter Maydell
Cc: qemu-stable
On 7/21/23 17:14, Michael Tokarev wrote:
> 19.07.2023 18:52, Helge Deller wrote:
>> qemu-user crashes immediately when running static binaries on the armhf
>> architecture. The problem is the memory layout where the executable is
>> loaded before the interpreter library, in which case the reserved brk
>> region clashes with the interpreter code and is released before qemu
>> tries to start the program.
>>
>> At load time qemu calculates a brk value for interpreter and executable
>> each. The fix is to choose the higher one of both.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> Cc: Andreas Schwab <schwab@suse.de>
>> Cc: qemu-stable@nongnu.org
>> Reported-by: Venkata.Pyla@toshiba-tsip.com
>> Closes: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1040981
>> ---
>> linux-user/elfload.c | 7 +++++++
>> 1 file changed, 7 insertions(+)
>>
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index a26200d9f3..94951630b1 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -3615,6 +3615,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;
>> + }
>
> So, this is kinda amusing.
> This broke arm64, ppc64el and s390x:
>
> arm64$ ./qemu-aarch64 /bin/sh -c '/bin/ls -dCFl *[t]* >/dev/null'
> qemu: uncaught target signal 11 (Segmentation fault) - core dumped
> Segmentation fault
>
> (it was just a quick test from debian qemu-user package).
>
> Reverting this patch makes it work again..
It seems the whole loading of binaries on aarch64 is somewhat wrong.
My patch just triggers the whole thing to blow up.
This is how the memory-map looks on physical hardware:
deller@amdahl:~$ uname -a
Linux amdahl 5.10.0-23-arm64 #1 SMP Debian 5.10.179-2 (2023-07-14) aarch64 GNU/Linux
deller@amdahl:~$ cat /proc/self/maps
aaaaafb70000-aaaaafb78000 r-xp 00000000 fe:02 131743 /bin/cat
aaaaafb88000-aaaaafb89000 r--p 00008000 fe:02 131743 /bin/cat
aaaaafb89000-aaaaafb8a000 rw-p 00009000 fe:02 131743 /bin/cat
aaaae022b000-aaaae024c000 rw-p 00000000 00:00 0 [heap]
ffff9ce78000-ffff9ce9a000 rw-p 00000000 00:00 0
ffff9ce9a000-ffff9cff6000 r-xp 00000000 fe:02 790863 /lib/aarch64-linux-gnu/libc-2.31.so
ffff9cff6000-ffff9d005000 ---p 0015c000 fe:02 790863 /lib/aarch64-linux-gnu/libc-2.31.so
ffff9d005000-ffff9d009000 r--p 0015b000 fe:02 790863 /lib/aarch64-linux-gnu/libc-2.31.so
ffff9d009000-ffff9d00b000 rw-p 0015f000 fe:02 790863 /lib/aarch64-linux-gnu/libc-2.31.so
ffff9d00b000-ffff9d00e000 rw-p 00000000 00:00 0
ffff9d00e000-ffff9d02f000 r-xp 00000000 fe:02 790820 /lib/aarch64-linux-gnu/ld-2.31.so
ffff9d033000-ffff9d035000 rw-p 00000000 00:00 0
ffff9d03c000-ffff9d03e000 r--p 00000000 00:00 0 [vvar]
ffff9d03e000-ffff9d03f000 r-xp 00000000 00:00 0 [vdso]
ffff9d03f000-ffff9d040000 r--p 00021000 fe:02 790820 /lib/aarch64-linux-gnu/ld-2.31.so
ffff9d040000-ffff9d042000 rw-p 00022000 fe:02 790820 /lib/aarch64-linux-gnu/ld-2.31.so
ffffe9ea3000-ffffe9ec4000 rw-p 00000000 00:00 0 [stack]
this is on qemu-linux-user-aarch64:
I have no name!@paq:/# cat /proc/self/maps
5500000000-5500009000 r-xp 00000000 08:01 2380521 /usr/bin/cat
5500009000-550001f000 ---p 00000000 00:00 0
550001f000-5500020000 r--p 0000f000 08:01 2380521 /usr/bin/cat
5500020000-5500021000 rw-p 00010000 08:01 2380521 /usr/bin/cat
5500021000-5500042000 rw-p 00000000 00:00 0
5502021000-5502022000 ---p 00000000 00:00 0
5502022000-5502822000 rw-p 00000000 00:00 0 [stack]
5502822000-5502848000 r-xp 00000000 08:01 2382563 /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1
5502848000-5502860000 ---p 00000000 00:00 0
5502860000-5502862000 r--p 0002e000 08:01 2382563 /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1
5502862000-5502864000 rw-p 00030000 08:01 2382563 /usr/lib/aarch64-linux-gnu/ld-linux-aarch64.so.1
5502864000-5502865000 r-xp 00000000 00:00 0
5502865000-5502867000 rw-p 00000000 00:00 0
5502870000-55029f7000 r-xp 00000000 08:01 2382566 /usr/lib/aarch64-linux-gnu/libc.so.6
55029f7000-5502a0d000 ---p 00187000 08:01 2382566 /usr/lib/aarch64-linux-gnu/libc.so.6
5502a0d000-5502a10000 r--p 0018d000 08:01 2382566 /usr/lib/aarch64-linux-gnu/libc.so.6
5502a10000-5502a12000 rw-p 00190000 08:01 2382566 /usr/lib/aarch64-linux-gnu/libc.so.6
5502a12000-5502a1f000 rw-p 00000000 00:00 0
Here I got:
interp_info.brk 0x5502863358
info->brk 0x5500020458
diff 0x2842f00 40 MB
I think we need to make sure that the shared libs (ld, glibc) gets loaded
at the top of the memory to free up heap space. Right now (without my patch)
there were 40MB heap, with my patch if clashed with ld.so.
Helge
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PULL 0/5] Linux user brk fixes patches
2023-07-19 15:52 [PULL 0/5] Linux user brk fixes patches Helge Deller
` (4 preceding siblings ...)
2023-07-19 15:52 ` [PULL 5/5] linux-user: Fix qemu-arm to run static armhf binaries Helge Deller
@ 2023-07-21 9:51 ` Peter Maydell
5 siblings, 0 replies; 12+ messages in thread
From: Peter Maydell @ 2023-07-21 9:51 UTC (permalink / raw)
To: Helge Deller
Cc: Richard Henderson, Laurent Vivier, Andreas Schwab,
Michael Tokarev, qemu-devel
On Wed, 19 Jul 2023 at 16:53, Helge Deller <deller@gmx.de> wrote:
>
> The following changes since commit 361d5397355276e3007825cc17217c1e4d4320f7:
>
> Merge tag 'block-pull-request' of https://gitlab.com/stefanha/qemu into staging (2023-07-17 15:49:27 +0100)
>
> are available in the Git repository at:
>
> https://github.com/hdeller/qemu-hppa.git tags/linux-user-brk-fixes-pull-request
>
> for you to fetch changes up to 518f32221af759a29500ac172c4c857bef142067:
>
> linux-user: Fix qemu-arm to run static armhf binaries (2023-07-18 20:42:05 +0200)
>
> ----------------------------------------------------------------
> linux-user: brk() syscall fixes and armhf static binary fix
>
> Commit 86f04735ac ("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 d28b3c90cfad ("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
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/8.1
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 12+ messages in thread