Re: [PATCH] mkimage: fix segfault on MacOS arm64

On 2 Dec 2021, at 22:16, Sergey V. Lobanov sergey@lobanov.in wrote:
mkimage segfaults due ASLR mechasim on MacOS arm64
It is required to use _dyld_get_image_vmaddr_slide() to prevent segfault on MacOS arm64
This patch ased on the discussion https://github.com/u-boot/u-boot/commit/3b142045e8a7f0ab17b6099e9226296af459...
Thanks to Ronny Kotzschmar and ptpt52 github user
Signed-off-by: Sergey V. Lobanov sergey@lobanov.in
tools/imagetool.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/imagetool.h b/tools/imagetool.h index e229a34ffc..13775ff9b3 100644 --- a/tools/imagetool.h +++ b/tools/imagetool.h @@ -271,11 +271,16 @@ int rockchip_copy_image(int fd, struct image_tool_params *mparams);
- b) we need a API call to get the respective section symbols */
#if defined(__MACH__) #include <mach-o/getsect.h> +#include <mach-o/dyld.h>
-#define INIT_SECTION(name) do { \ +#define INIT_SECTION(name) struct image_type_params \
- **__cat(__start_, name), **__cat(__stop_, name); \
This change alters the interface of INIT_SECTION. Previously it was just required that something had called it before you referenced the start/stop symbols. Now there are two things going on:
1. Any references have to be in a scope that can see the INIT_SECTION call. 2. This is no longer a single statement, so if (foo) INIT_SECTION(name); breaks.
I don’t see why this change is needed, either. It should still be idempotent to set the global multiple times even after your change to add the base address, since that is done to the temporary local variable.
- do { \ unsigned long name ## _len; \ char *__cat(pstart_, name) = getsectdata("__DATA", \ #name, &__cat(name, _len)); \
__cat(pstart_, name) += \
_dyld_get_image_vmaddr_slide(0); \
Your formatting here is broken, you have an extra tab on both lines.
Jess
char *__cat(pstop_, name) = __cat(pstart_, name) + \ __cat(name, _len); \ __cat(__start_, name) = (void *)__cat(pstart_, name); \
@@ -283,7 +288,6 @@ int rockchip_copy_image(int fd, struct image_tool_params *mparams); } while (0) #define SECTION(name) __attribute__((section("__DATA, " #name)))
-struct image_type_params **__start_image_type, **__stop_image_type; #else #define INIT_SECTION(name) /* no-op for ELF */
#define SECTION(name) __attribute__((section(#name)))
2.30.1 (Apple Git-130)

Thanks a lot for your review, I’ve sent PATCH v2 with the changes related to your comments
https://lists.denx.de/pipermail/u-boot/2022-January/472133.html
On 11 Jan 2022, at 22:42, Jessica Clarke jrtc27@jrtc27.com wrote:
On 2 Dec 2021, at 22:16, Sergey V. Lobanov sergey@lobanov.in wrote:
mkimage segfaults due ASLR mechasim on MacOS arm64
It is required to use _dyld_get_image_vmaddr_slide() to prevent segfault on MacOS arm64
This patch ased on the discussion https://github.com/u-boot/u-boot/commit/3b142045e8a7f0ab17b6099e9226296af459...
Thanks to Ronny Kotzschmar and ptpt52 github user
Signed-off-by: Sergey V. Lobanov sergey@lobanov.in
tools/imagetool.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/tools/imagetool.h b/tools/imagetool.h index e229a34ffc..13775ff9b3 100644 --- a/tools/imagetool.h +++ b/tools/imagetool.h @@ -271,11 +271,16 @@ int rockchip_copy_image(int fd, struct image_tool_params *mparams);
- b) we need a API call to get the respective section symbols */
#if defined(__MACH__) #include <mach-o/getsect.h> +#include <mach-o/dyld.h>
-#define INIT_SECTION(name) do { \ +#define INIT_SECTION(name) struct image_type_params \
- **__cat(__start_, name), **__cat(__stop_, name); \
This change alters the interface of INIT_SECTION. Previously it was just required that something had called it before you referenced the start/stop symbols. Now there are two things going on:
- Any references have to be in a scope that can see the INIT_SECTION
call. 2. This is no longer a single statement, so if (foo) INIT_SECTION(name); breaks.
I don’t see why this change is needed, either. It should still be idempotent to set the global multiple times even after your change to add the base address, since that is done to the temporary local variable.
- do { \ unsigned long name ## _len; \ char *__cat(pstart_, name) = getsectdata("__DATA", \ #name, &__cat(name, _len)); \
__cat(pstart_, name) += \
_dyld_get_image_vmaddr_slide(0); \
Your formatting here is broken, you have an extra tab on both lines.
Jess
char *__cat(pstop_, name) = __cat(pstart_, name) + \ __cat(name, _len); \ __cat(__start_, name) = (void *)__cat(pstart_, name); \
@@ -283,7 +288,6 @@ int rockchip_copy_image(int fd, struct image_tool_params *mparams); } while (0) #define SECTION(name) __attribute__((section("__DATA, " #name)))
-struct image_type_params **__start_image_type, **__stop_image_type; #else #define INIT_SECTION(name) /* no-op for ELF */
#define SECTION(name) __attribute__((section(#name)))
2.30.1 (Apple Git-130)
participants (2)
-
Jessica Clarke
-
Sergey V. Lobanov