[patch] 0001-libsandbox-try-harder-not-to-regenerate-environment.patch

0001-libsandbox-try-harder-not-to-regenerate-environment.patch (text/plain), 2.88 KB, created by Sergei Trofimovich (RETIRED) on 2021-03-13 22:46:09 UTC

From b0e52c0a4bee0ddf4e29488fa884c830430f2ddc Mon Sep 17 00:00:00 2001
From: Sergei Trofimovich <[e-mail]>
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 <[e-mail]>
---
 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