From b0e52c0a4bee0ddf4e29488fa884c830430f2ddc Mon Sep 17 00:00:00 2001 From: Sergei Trofimovich Date: Sat, 13 Mar 2021 22:35:53 +0000 Subject: [PATCH] libsandbox: try harder not to regenerate environment In bug #775842 Sam noticed sandbox crash on libabigail-1.8.2 testsuite: #0 0x00007f0e0d10e392 in sb_new_envp () at libsandbox.c:1184 #1 0x00007f0e0d112745 in system_DEFAULT () at wrapper-funcs/__wrapper_exec.c:315 #2 0x000055ece6570b52 in test_task::perform (this=0x55ece72978b0) at /usr/lib/gcc/x86_64-pc-linux-gnu/11.0.1/include/g++-v11/bits/basic_string.h:186 #3 0x00007f0e0d05fce5 in abigail::workers::worker::wait_to_execute_a_task (p=0x55ece728f380) at /tmp/portage/dev-util/libabigail-1.8.2/work/libabigail-1.8.2/src/abg-workers.cc:415 #4 0x00007f0e0c8f7cae in start_thread (arg=0x7f0e0b433640) at pthread_create.c:473 #5 0x00007f0e0ca0eb2f in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95 The test roughly does call system() from spawned parallel threads: for_each_test(){ spawn_thread([]{ system("cmd"); wait(); } } Sandbox has to inject sandbox-specific environment variables where they don't exist. This is fundamentally racy in multithreaded environment: for_each_test(){ spawn_thread([]{ environ = modified_env; system("cmd"); wait(); environ = orig_env; } } Most of the time environment does not have to change after initial environment injection. f3e51a9303124 ("exec*() wrappers: never mutate 'environ' of host process") exposed a bug in sandbox's logic of checking existing environment: unset variables like `SANDBOX_DENY` The change treats unset/expected-unset variables as non deviating. Reported-by: Sam James Bug: https://bugs.gentoo.org/775842 Signed-off-by: Sergei Trofimovich --- libsandbox/libsandbox.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/libsandbox/libsandbox.c b/libsandbox/libsandbox.c index b4d732d..166516c 100644 --- a/libsandbox/libsandbox.c +++ b/libsandbox/libsandbox.c @@ -1181,6 +1181,7 @@ struct sb_envp_ctx sb_new_envp(char **envp, bool insert) found_var_cnt = 0; memset(found_vars, 0, sizeof(found_vars)); + /* Iterate through user's environment and check against expected. */ str_list_for_each_item(envp, entry, count) { for (i = 0; i < num_vars; ++i) { if (found_vars[i]) @@ -1192,6 +1193,14 @@ struct sb_envp_ctx sb_new_envp(char **envp, bool insert) } } + /* Treat unset and expected-unset variables as found. This will allow us + * to keep existing environment. */ + for (i = 0; i < num_vars; ++i) { + if (vars[i].value == NULL && found_vars[i] == NULL) { + ++found_var_cnt; + } + } + /* Now specially handle merging of LD_PRELOAD */ char *ld_preload; bool merge_ld_preload = found_vars[0] && !strstr(found_vars[0], sandbox_lib); -- 2.30.2