From be954f53aa16ff04c54faca792c49b23f48163d8 Mon Sep 17 00:00:00 2001 From: Martin Kampas Date: Thu, 10 Dec 2020 09:05:21 +0100 Subject: [PATCH] [sb2] prepare_exec: Do not discard changes to environment. JB#52287 Initially, my_envp equals to *new_envp, then it is modified. Those modifications are lost when *new_envp is used after that. The (possibly) discarted modifications to env. variables affect SBOX_DISABLE_MAPPING, SBOX_DISABLE_ARGVENVP and __SB2_REAL_BINARYNAME. Regarding SBOX_DISABLE_MAPPING, consider a host executable being executed by sb2 and trying to execute another host executable. With mapping not disabled, the other executable may be mapped to a different executable under target/tools root instead. In most cases this is likely to happen unnoticed. It breaks when there is an incompatibility between those, e.g., when the host executable is a GCC compiler trying to execute some of its internal tools - an i486 host compiler may end up executing an arm assembler this way. Trying to explain why *new_envp was used here, I don't believe it was a typo. At the same time I don't believe it was intentional to change the logic this way. I can't think of a reason to discard changes to the afforementioned env. vars. To me it seems inevitable to keep them set. Also, change was introduced with commit 3f042f363e116976dcc9c43c658a9ce3f889f2f3 (Execs: Postprocess native dynamic binaries with C code) whose only objective seems to be reimplementation of some Lua code in C. Maybe it was done to deal with some compiler bug and no one noticed it breaks things, as this only breaks in rather special cases like the one mentioned above. --- scratchbox2/execs/sb_exec.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scratchbox2/execs/sb_exec.c b/scratchbox2/execs/sb_exec.c index c38ba92c..3c00f1bd 100644 --- a/scratchbox2/execs/sb_exec.c +++ b/scratchbox2/execs/sb_exec.c @@ -1535,7 +1535,7 @@ static int prepare_exec(const char *exec_fn_name, exec_policy_name, &mapped_file, &my_file, binaryname, (const char **)my_argv, &my_new_argv, - (const char **)*new_envp, &my_new_envp, + (const char **)my_envp, &my_new_envp, "target"); } else { postprocess_result = exec_postprocess_native_executable( @@ -1543,7 +1543,7 @@ static int prepare_exec(const char *exec_fn_name, &mapped_file, &my_file, binaryname, &info, (const char **)my_argv, &my_new_argv, - (const char **)*new_envp, &my_new_envp); + (const char **)my_envp, &my_new_envp); } my_envp = (char**)my_new_envp; /* FIXME */ my_argv = (char**)my_new_argv; /* FIXME */ @@ -1570,7 +1570,7 @@ static int prepare_exec(const char *exec_fn_name, exec_policy_name, &mapped_file, &my_file, binaryname, (const char **)my_argv, &my_new_argv, - (const char **)*new_envp, &my_new_envp, + (const char **)my_envp, &my_new_envp, "native"); my_envp = (char**)my_new_envp; /* FIXME */ my_argv = (char**)my_new_argv; /* FIXME */ @@ -1609,7 +1609,7 @@ static int prepare_exec(const char *exec_fn_name, exec_policy_name, &mapped_file, &my_file, binaryname, (const char **)my_argv, &my_new_argv, - (const char **)*new_envp, &my_new_envp); + (const char **)my_envp, &my_new_envp); my_envp = (char**)my_new_envp; /* FIXME */ my_argv = (char**)my_new_argv; /* FIXME */ } @@ -1639,7 +1639,7 @@ static int prepare_exec(const char *exec_fn_name, postprocess_result = exec_postprocess_cpu_transparency_executable( exec_policy_name, &mapped_file, &my_file, binaryname, - (const char **)my_argv, &my_new_argv, (const char **)*new_envp, &my_new_envp, + (const char **)my_argv, &my_new_argv, (const char **)my_envp, &my_new_envp, "target"); my_envp = (char**)my_new_envp; /* FIXME */ my_argv = (char**)my_new_argv; /* FIXME */