[gentoo-commits] proj/sandbox:master commit in: libsandbox/

[Sergei Trofimovich]

commit:     b321cd403c653d5bac54b5ec8341bc631fe3331e
Author:     Sergei Trofimovich <slyfox <AT> gentoo <DOT> org>
AuthorDate: Sat Mar 13 22:35:53 2021 +0000
Commit:     Sergei Trofimovich <slyfox <AT> gentoo <DOT> org>
CommitDate: Sat Mar 13 22:53:39 2021 +0000
URL:        https://gitweb.gentoo.org/proj/sandbox.git/commit/?id=b321cd40

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 <slyfox <AT> gentoo.org>

 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);