diff mbox series

[3/6] linux-user: Adjust brk for load_bias

Message ID 20230816181437.572997-4-richard.henderson@linaro.org
State Superseded
Headers show
Series linux-user: Rewrite open_self_maps | expand

Commit Message

Richard Henderson Aug. 16, 2023, 6:14 p.m. UTC
PIE executables are usually linked at offset 0 and are
relocated somewhere during load.  The hiaddr needs to
be adjusted to keep the brk next to the executable.

Cc: qemu-stable@nongnu.org
Fixes: 1f356e8c013 ("linux-user: Adjust initial brk when interpreter is close to executable")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Philippe Mathieu-Daudé Aug. 17, 2023, 8:53 a.m. UTC | #1
On 16/8/23 20:14, Richard Henderson wrote:
> PIE executables are usually linked at offset 0 and are
> relocated somewhere during load.  The hiaddr needs to
> be adjusted to keep the brk next to the executable.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 1f356e8c013 ("linux-user: Adjust initial brk when interpreter is close to executable")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   linux-user/elfload.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index ccfbf82836..ab11f141c3 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -3278,7 +3278,7 @@ static void load_elf_image(const char *image_name, const ImageSource *src,
>       info->start_data = -1;
>       info->end_data = 0;
>       /* Usual start for brk is after all sections of the main executable. */
> -    info->brk = TARGET_PAGE_ALIGN(hiaddr);
> +    info->brk = TARGET_PAGE_ALIGN(hiaddr + load_bias);

Did you got some odd behavior or figured that by
code review?

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Michael Tokarev Aug. 17, 2023, 4:04 p.m. UTC | #2
16.08.2023 21:14, Richard Henderson wrote:
> PIE executables are usually linked at offset 0 and are
> relocated somewhere during load.  The hiaddr needs to
> be adjusted to keep the brk next to the executable.
> 
> Cc: qemu-stable@nongnu.org
> Fixes: 1f356e8c013 ("linux-user: Adjust initial brk when interpreter is close to executable")

FWIW, 1f356e8c013 is v8.1.0-rc2-86, - why did you Cc qemu-stable@?

If this "Adjust brk for load_bias" fix isn't supposed to be part of 8.1.0 release,
sure thing I'll pick it up for stable-8.1, but it looks like it should be in 8.1.0.

Or are you saying 1f356e8c013 should be picked for stable-8.0, together with this one?

(We're yet to decide if stable-8.0 should have any recent linux-user changes).

/mjt
Richard Henderson Aug. 18, 2023, 12:16 a.m. UTC | #3
On 8/17/23 01:53, Philippe Mathieu-Daudé wrote:
> On 16/8/23 20:14, Richard Henderson wrote:
>> PIE executables are usually linked at offset 0 and are
>> relocated somewhere during load.  The hiaddr needs to
>> be adjusted to keep the brk next to the executable.
>>
>> Cc: qemu-stable@nongnu.org
>> Fixes: 1f356e8c013 ("linux-user: Adjust initial brk when interpreter is close to 
>> executable")
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>   linux-user/elfload.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
>> index ccfbf82836..ab11f141c3 100644
>> --- a/linux-user/elfload.c
>> +++ b/linux-user/elfload.c
>> @@ -3278,7 +3278,7 @@ static void load_elf_image(const char *image_name, const 
>> ImageSource *src,
>>       info->start_data = -1;
>>       info->end_data = 0;
>>       /* Usual start for brk is after all sections of the main executable. */
>> -    info->brk = TARGET_PAGE_ALIGN(hiaddr);
>> +    info->brk = TARGET_PAGE_ALIGN(hiaddr + load_bias);
> 
> Did you got some odd behavior or figured that by
> code review?
> 
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Odd behaviour, easily seen by [heap] being weird or missing.


r~
Richard Henderson Aug. 18, 2023, 12:17 a.m. UTC | #4
On 8/17/23 09:04, Michael Tokarev wrote:
> 16.08.2023 21:14, Richard Henderson wrote:
>> PIE executables are usually linked at offset 0 and are
>> relocated somewhere during load.  The hiaddr needs to
>> be adjusted to keep the brk next to the executable.
>>
>> Cc: qemu-stable@nongnu.org
>> Fixes: 1f356e8c013 ("linux-user: Adjust initial brk when interpreter is close to 
>> executable")
> 
> FWIW, 1f356e8c013 is v8.1.0-rc2-86, - why did you Cc qemu-stable@?
> 
> If this "Adjust brk for load_bias" fix isn't supposed to be part of 8.1.0 release,
> sure thing I'll pick it up for stable-8.1, but it looks like it should be in 8.1.0.
> 
> Or are you saying 1f356e8c013 should be picked for stable-8.0, together with this one?
> 
> (We're yet to decide if stable-8.0 should have any recent linux-user changes).

This has missed 8.1.0-rc4 and therefore will not be in 8.1.0.
I have tagged it stable for stable-8.1 for 8.1.1.


r~
Dominique MARTINET Nov. 26, 2024, 7:11 a.m. UTC | #5
This commit is fairly old, but this appears to cause a segfault for
older versions of ldconfig:
```
$ docker run --rm --platform linux/arm64/v8 -ti docker.io/debian:bullseye-slim ldconfig
qemu: uncaught target signal 11 (Segmentation fault) - core dumped
Segmentation fault (core dumped)
```

The segfault happens inside ldconfig code (code_gen_buffer in qemu's
backtrace), so I'm not sure how to debug that further, but it doesn't
reproduce in bookworm's ldconfig so that is something that was "fixed"
in glibc at some point.

If someone needs to run older debian releases with a newer qemu that
might be a problem in the future?

[we might need to run old containers once every few years to rebuild old
projects in a similar environment they were built on, so would
eventually need to work around this problem somehow]


The failure can be reproduced just running `qemu-aarch64
./path/to/ldconfig` on an extracted container so it was easy to bisect
and I've got down to this commit; hence replying here directly with
involved people.
------
commit aec338d63bc28f1f13d5e64c561d7f1dd0e4b07e
Author: Richard Henderson <richard.henderson@linaro.org>
Date:   Wed Aug 16 10:32:18 2023 -0700

    linux-user: Adjust brk for load_bias
    
    PIE executables are usually linked at offset 0 and are
    relocated somewhere during load.  The hiaddr needs to
    be adjusted to keep the brk next to the executable.
    
    Cc: qemu-stable@nongnu.org
    Fixes: 1f356e8c013 ("linux-user: Adjust initial brk when interpreter is close to executable")
    Tested-by: Helge Deller <deller@gmx.de>
    Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>
    Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
    Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
------

I've done my share of debugging linux-user last week[1] so I'll leave this
as is for now, I've downgraded to (a non-static-pie build of) 7.1 for
our build machine and am not in immediate trouble.
[1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1053101

If this doesn't get much interest I might try to pick at it further in
a couple of weeks, assuming it's something we can/want to fix on qemu
side.

Thanks,
Ilya Leoshkevich Nov. 26, 2024, 9:24 a.m. UTC | #6
On Tue, 2024-11-26 at 16:11 +0900, Dominique MARTINET wrote:
> This commit is fairly old, but this appears to cause a segfault for
> older versions of ldconfig:
> ```
> $ docker run --rm --platform linux/arm64/v8 -ti
> docker.io/debian:bullseye-slim ldconfig
> qemu: uncaught target signal 11 (Segmentation fault) - core dumped
> Segmentation fault (core dumped)
> ```
> 
> The segfault happens inside ldconfig code (code_gen_buffer in qemu's
> backtrace), so I'm not sure how to debug that further, but it doesn't
> reproduce in bookworm's ldconfig so that is something that was
> "fixed"
> in glibc at some point.
> 
> If someone needs to run older debian releases with a newer qemu that
> might be a problem in the future?
> 
> [we might need to run old containers once every few years to rebuild
> old
> projects in a similar environment they were built on, so would
> eventually need to work around this problem somehow]
> 
> 
> The failure can be reproduced just running `qemu-aarch64
> ./path/to/ldconfig` on an extracted container so it was easy to
> bisect
> and I've got down to this commit; hence replying here directly with
> involved people.
> ------
> commit aec338d63bc28f1f13d5e64c561d7f1dd0e4b07e
> Author: Richard Henderson <richard.henderson@linaro.org>
> Date:   Wed Aug 16 10:32:18 2023 -0700
> 
>     linux-user: Adjust brk for load_bias
>     
>     PIE executables are usually linked at offset 0 and are
>     relocated somewhere during load.  The hiaddr needs to
>     be adjusted to keep the brk next to the executable.
>     
>     Cc: qemu-stable@nongnu.org
>     Fixes: 1f356e8c013 ("linux-user: Adjust initial brk when
> interpreter is close to executable")
>     Tested-by: Helge Deller <deller@gmx.de>
>     Reviewed-by: Ilya Leoshkevich <iii@linux.ibm.com>
>     Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>     Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ------
> 
> I've done my share of debugging linux-user last week[1] so I'll leave
> this
> as is for now, I've downgraded to (a non-static-pie build of) 7.1 for
> our build machine and am not in immediate trouble.
> [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=1053101
> 
> If this doesn't get much interest I might try to pick at it further
> in
> a couple of weeks, assuming it's something we can/want to fix on qemu
> side.
> 
> Thanks,

Hi,

I think this is
https://gitlab.com/qemu-project/qemu/-/issues/1913

Best regards,
Ilya
Dominique MARTINET Nov. 26, 2024, 9:29 a.m. UTC | #7
Ilya Leoshkevich wrote on Tue, Nov 26, 2024 at 10:24:12AM +0100:
> I think this is
> https://gitlab.com/qemu-project/qemu/-/issues/1913

Thank you, I should have looked there first !

I'll continue to follow-up on the issue if time permits as it doesn't
look like this has had progress in the past few months
diff mbox series

Patch

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index ccfbf82836..ab11f141c3 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -3278,7 +3278,7 @@  static void load_elf_image(const char *image_name, const ImageSource *src,
     info->start_data = -1;
     info->end_data = 0;
     /* Usual start for brk is after all sections of the main executable. */
-    info->brk = TARGET_PAGE_ALIGN(hiaddr);
+    info->brk = TARGET_PAGE_ALIGN(hiaddr + load_bias);
     info->elf_flags = ehdr->e_flags;
 
     prot_exec = PROT_EXEC;