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

[Jessica Clarke <jrtc27@jrtc27.com>]

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/3b142045e8a7f0ab17b6099e9226296af45967d0
> 
> 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)