From 0d7f3068981f2b08e583cec21d9069e97c73addd Mon Sep 17 00:00:00 2001 From: Simon Ruderich Date: Fri, 21 Jun 2013 18:26:23 +0200 Subject: [PATCH] Overwrite tracked file descriptors when exporting COLORED_STDERR_FDS. If the list of tracked file descriptors is modified to contain unwanted values (e.g. when a parent process closes or redirects stderr), this makes it possible to set the tracked file descriptors back to a given list. This problem occurred with startx which somehow closes or redirects stderr. As COLORED_STDERR_FDS was set in the shell running startx, this disabled coloring stderr for all processes started in the X11 session. This is no longer an issue. As COLORED_STDERR_FDS is set in the shell configuration file, it automatically sets the list to the correct value when a shell is opened in the terminal emulator once X11 is running. --- README | 4 +++ src/coloredstderr.c | 65 +++++++++++++++++++++++-------------- src/constants.h | 1 + src/trackfds.h | 31 +++++++++++++++--- tests/example_exec.c | 35 ++++++++++++++++++-- tests/example_exec.expected | 57 +++++++++++++++++++------------- tests/lib.sh | 9 +++-- tests/test_environment.sh | 17 +++++++++- 8 files changed, 160 insertions(+), 59 deletions(-) diff --git a/README b/README index 967a6fd..79a4084 100644 --- a/README +++ b/README @@ -95,6 +95,10 @@ The following additional environment variables are available: terminal, e.g. when writing to a file. By default, only writes to a terminal are colored. +All environment variables starting with 'COLORED_STDERR_PRIVATE_*' are +internal variables used by the implementation and should not be set manually. +See the source for details. + To set custom colors as pre/post strings you can use the `$''` feature of Bash and Zsh: diff --git a/src/coloredstderr.c b/src/coloredstderr.c index 00de8a5..586b024 100644 --- a/src/coloredstderr.c +++ b/src/coloredstderr.c @@ -78,6 +78,9 @@ static size_t (*real_fwrite)(void const *, size_t, size_t, FILE *); static int initialized; /* Force hooked writes even when not writing to a tty. Used for tests. */ static int force_write_to_non_tty; +/* Was ENV_NAME_FDS found and used when init_from_environment() was called? + * This is not true if the process set it manually after initialization. */ +static int used_fds_set_by_user; #include "constants.h" @@ -508,34 +511,23 @@ pid_t vfork(void) { /* Hook execve() and the other exec*() functions. Some shells use exec*() with * a custom environment which doesn't necessarily contain our updates to - * ENV_NAME_FDS. It's also faster to update the environment only when - * necessary, right before the exec() to pass it to the new process. */ + * ENV_NAME_PRIVATE_FDS. It's also faster to update the environment only when + * necessary, right before the exec(), to pass it to the new program. */ /* int execve(char const *, char * const [], char * const []) */ HOOK_FUNC_DEF3(int, execve, char const *, filename, char * const *, argv, char * const *, env) { DLSYM_FUNCTION(real_execve, "execve"); - int found = 0; - size_t index = 0; - - /* Count arguments and search for existing ENV_NAME_FDS environment - * variable. */ + /* Count environment variables. */ size_t count = 0; char * const *x = env; - while (*x) { - if (!strncmp(*x, ENV_NAME_FDS "=", strlen(ENV_NAME_FDS) + 1)) { - found = 1; - index = count; - } - - x++; + while (*x++) { count++; } /* Terminating NULL. */ count++; char *env_copy[count + 1 /* space for our new entry if necessary */]; - memcpy(env_copy, env, count * sizeof(char *)); /* Make sure the information from the environment is loaded. We can't just * do nothing (like update_environment()) because the caller might pass a @@ -544,17 +536,40 @@ HOOK_FUNC_DEF3(int, execve, char const *, filename, char * const *, argv, char * init_from_environment(); } - char fds_env[strlen(ENV_NAME_FDS) + 1 + update_environment_buffer_size()]; - strcpy(fds_env, ENV_NAME_FDS "="); - update_environment_buffer(fds_env + strlen(ENV_NAME_FDS) + 1); + char fds_env[strlen(ENV_NAME_PRIVATE_FDS) + + 1 + update_environment_buffer_size()]; + strcpy(fds_env, ENV_NAME_PRIVATE_FDS "="); + update_environment_buffer(fds_env + strlen(ENV_NAME_PRIVATE_FDS) + 1); - if (found) { - env_copy[index] = fds_env; - } else { - /* If the process removed ENV_NAME_FDS from the environment, re-add - * it. */ - env_copy[count-1] = fds_env; - env_copy[count] = NULL; + int found = 0; + char **x_copy = env_copy; + + /* Copy the environment manually; allows skipping elements. */ + x = env; + while ((*x_copy = *x)) { + /* Remove ENV_NAME_FDS if we've already used its value. The new + * program must use the updated list from ENV_NAME_PRIVATE_FDS. */ + if (used_fds_set_by_user + && !strncmp(*x, ENV_NAME_FDS "=", strlen(ENV_NAME_FDS) + 1)) { + x++; + continue; + /* Update ENV_NAME_PRIVATE_FDS. */ + } else if (!strncmp(*x, ENV_NAME_PRIVATE_FDS "=", + strlen(ENV_NAME_PRIVATE_FDS) + 1)) { + *x_copy = fds_env; + found = 1; + } + + x++; + x_copy++; + } + /* The loop "condition" NULL-terminates env_copy. */ + + if (!found) { + /* If the process removed ENV_NAME_PRIVATE_FDS from the environment, + * re-add it. */ + *x_copy++ = fds_env; + *x_copy++ = NULL; } return real_execve(filename, argv, env_copy); diff --git a/src/constants.h b/src/constants.h index f195cc6..4cc69ca 100644 --- a/src/constants.h +++ b/src/constants.h @@ -25,6 +25,7 @@ #define ENV_NAME_PRE_STRING "COLORED_STDERR_PRE" #define ENV_NAME_POST_STRING "COLORED_STDERR_POST" #define ENV_NAME_FORCE_WRITE "COLORED_STDERR_FORCE_WRITE" +#define ENV_NAME_PRIVATE_FDS "COLORED_STDERR_PRIVATE_FDS" /* Strings written before/after each matched function. */ #define DEFAULT_PRE_STRING "\033[91m" diff --git a/src/trackfds.h b/src/trackfds.h index f0ba879..30693f4 100644 --- a/src/trackfds.h +++ b/src/trackfds.h @@ -72,8 +72,9 @@ static int init_tracked_fds_list(size_t count) { /* Load tracked file descriptors from the environment. The environment is used * to pass the information to child processes. * - * ENV_NAME_FDS has the following format: Each descriptor as string followed - * by a comma; there's a trailing comma. Example: "2,4,". */ + * ENV_NAME_FDS and ENV_NAME_PRIVATE_FDS have the following format: Each + * descriptor as string followed by a comma; there's a trailing comma. + * Example: "2,4,". */ static void init_from_environment(void) { #ifdef DEBUG debug("init_from_environment()\t\t[%d]\n", getpid()); @@ -92,14 +93,23 @@ static void init_from_environment(void) { force_write_to_non_tty = 1; } + /* Prefer user defined list of file descriptors, fall back to file + * descriptors passed through the environment from the parent process. */ env = getenv(ENV_NAME_FDS); + if (env) { + used_fds_set_by_user = 1; + } else { + env = getenv(ENV_NAME_PRIVATE_FDS); + } if (!env) { errno = saved_errno; return; } #ifdef DEBUG debug(" getenv(\"%s\"): \"%s\"\n", ENV_NAME_FDS, env); + debug(" getenv(\"%s\"): \"%s\"\n", ENV_NAME_PRIVATE_FDS, env); #endif + /* Environment must be treated read-only. */ char env_copy[strlen(env) + 1]; strcpy(env_copy, env); @@ -225,16 +235,29 @@ static void update_environment(void) { return; } + int saved_errno = errno; + char env[update_environment_buffer_size()]; env[0] = 0; update_environment_buffer(env); #if 0 - debug(" setenv(\"%s\", \"%s\", 1)\n", ENV_NAME_FDS, env); + debug(" setenv(\"%s\", \"%s\", 1)\n", ENV_NAME_PRIVATE_FDS, env); #endif + setenv(ENV_NAME_PRIVATE_FDS, env, 1 /* overwrite */); - setenv(ENV_NAME_FDS, env, 1 /* overwrite */); + /* Child processes must use ENV_NAME_PRIVATE_FDS to get the updated list + * of tracked file descriptors, not the static list provided by the user + * in ENV_NAME_FDS. + * + * But only remove it if the static list in ENV_NAME_FDS was loaded by + * init_from_environment() and merged into ENV_NAME_PRIVATE_FDS. */ + if (used_fds_set_by_user) { + unsetenv(ENV_NAME_FDS); + } + + errno = saved_errno; } diff --git a/tests/example_exec.c b/tests/example_exec.c index bd56700..2d22ce0 100644 --- a/tests/example_exec.c +++ b/tests/example_exec.c @@ -169,9 +169,38 @@ int main(int argc unused, char **argv) { execvp(argv[0], args); return EXIT_FAILURE; - } else { + + /* Test handling of COLORED_STDERR_FDS. */ + + } else if (!skip--) { + /* And the rest. */ close(3); close(8); + + dup2(2, 5); + + char *args[] = { argv0, NULL }; + char *envp[] = { ldpreload, "COLORED_STDERR_FDS=5,", NULL }; + + execve(argv[0], args, envp); + return EXIT_FAILURE; + } else if (!skip--) { + char *args[] = { argv0, NULL }; + char *envp[] = { ldpreload, NULL }; + + dup2(5, 6); + close(5); + + execve(argv[0], args, envp); + return EXIT_FAILURE; + } else if (!skip--) { + close(6); + + char *args[] = { argv0, NULL }; + setenv("COLORED_STDERR_FDS", "2,", 1); + + execv(argv[0], args); + return EXIT_FAILURE; } @@ -292,13 +321,13 @@ int main(int argc unused, char **argv) { } else if (!skip--) { puts("argv[0] = |./example_exec|"); puts("environ[0] = |TEST=54|"); - puts("environ[2] = |COLORED_STDERR_FDS=2,|"); + puts("environ[2] = |COLORED_STDERR_PRIVATE_FDS=2,|"); puts(""); puts("argv[0] = |./example_exec|"); puts("argv[1] = |foo|"); puts("argv[2] = |bar|"); puts("environ[0] = |TEST=55|"); - puts("environ[2] = |COLORED_STDERR_FDS=2,|"); + puts("environ[2] = |COLORED_STDERR_PRIVATE_FDS=2,|"); puts(""); #endif } diff --git a/tests/example_exec.expected b/tests/example_exec.expected index c79a3de..61ca212 100644 --- a/tests/example_exec.expected +++ b/tests/example_exec.expected @@ -1,114 +1,125 @@ argv[0] = |./example_exec| -environ[1] = |COLORED_STDERR_FDS=2,| +environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,| CHECKING COLORING. argv[0] = |./example_exec| -environ[1] = |COLORED_STDERR_FDS=2,3,| +environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,3,| argv[0] = |./example_exec| -environ[1] = |COLORED_STDERR_FDS=2,3,4,| +environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,3,4,| argv[0] = |./example_exec| -environ[1] = |COLORED_STDERR_FDS=2,3,4,5,| +environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,3,4,5,| argv[0] = |./example_exec| -environ[1] = |COLORED_STDERR_FDS=2,3,4,5,6,| +environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,3,4,5,6,| argv[0] = |./example_exec| -environ[1] = |COLORED_STDERR_FDS=2,3,4,7,| +environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,3,4,7,| argv[0] = |./example_exec| -environ[1] = |COLORED_STDERR_FDS=2,3,8,| +environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,3,8,| + +argv[0] = |./example_exec| +environ[1] = |COLORED_STDERR_FDS=5,| +environ[2] = |COLORED_STDERR_PRIVATE_FDS=2,5,| + +argv[0] = |./example_exec| +environ[1] = |COLORED_STDERR_PRIVATE_FDS=6,| + +argv[0] = |./example_exec| +environ[1] = |COLORED_STDERR_PRIVATE_FDS=| +environ[2] = |COLORED_STDERR_FDS=2,| CHECKING TRANSPARENCY. argv[0] = |./example_exec| -environ[1] = |COLORED_STDERR_FDS=2,| +environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,| argv[0] = |./example_exec| environ[0] = |TEST=42| -environ[2] = |COLORED_STDERR_FDS=2,| +environ[2] = |COLORED_STDERR_PRIVATE_FDS=2,| argv[0] = |./example_exec| argv[1] = |foo| argv[2] = |bar| environ[0] = |TEST=43| environ[1] = |FOO=| -environ[3] = |COLORED_STDERR_FDS=2,| +environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,| argv[0] = |./example_exec| environ[0] = |TEST=44| environ[1] = |FOO=| -environ[3] = |COLORED_STDERR_FDS=2,| +environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,| argv[0] = |./example_exec| argv[1] = |foo| argv[2] = |bar| environ[0] = |TEST=45| environ[1] = |FOO=| -environ[3] = |COLORED_STDERR_FDS=2,| +environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,| argv[0] = |./example_exec| environ[0] = |TEST=46| environ[1] = |FOO=| -environ[3] = |COLORED_STDERR_FDS=2,| +environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,| argv[0] = |./example_exec| argv[1] = |foo| argv[2] = |bar| environ[0] = |TEST=47| environ[1] = |FOO=| -environ[3] = |COLORED_STDERR_FDS=2,| +environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,| argv[0] = |./example_exec| -environ[1] = |COLORED_STDERR_FDS=2,| +environ[1] = |COLORED_STDERR_PRIVATE_FDS=2,| argv[0] = |./example_exec| environ[0] = |TEST=48| -environ[2] = |COLORED_STDERR_FDS=2,| +environ[2] = |COLORED_STDERR_PRIVATE_FDS=2,| argv[0] = |./example_exec| argv[1] = |foo| argv[2] = |bar| environ[0] = |TEST=49| environ[1] = |FOO=| -environ[3] = |COLORED_STDERR_FDS=2,| +environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,| argv[0] = |./example_exec| environ[0] = |TEST=50| environ[1] = |FOO=| -environ[3] = |COLORED_STDERR_FDS=2,| +environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,| argv[0] = |./example_exec| argv[1] = |foo| argv[2] = |bar| environ[0] = |TEST=51| environ[1] = |FOO=| -environ[3] = |COLORED_STDERR_FDS=2,| +environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,| argv[0] = |./example_exec| environ[0] = |TEST=52| environ[1] = |FOO=| -environ[3] = |COLORED_STDERR_FDS=2,| +environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,| argv[0] = |./example_exec| argv[1] = |foo| argv[2] = |bar| environ[0] = |TEST=53| environ[1] = |FOO=| -environ[3] = |COLORED_STDERR_FDS=2,| +environ[3] = |COLORED_STDERR_PRIVATE_FDS=2,| argv[0] = |./example_exec| environ[0] = |TEST=54| -environ[2] = |COLORED_STDERR_FDS=2,| +environ[2] = |COLORED_STDERR_PRIVATE_FDS=2,| argv[0] = |./example_exec| argv[1] = |foo| argv[2] = |bar| environ[0] = |TEST=55| -environ[2] = |COLORED_STDERR_FDS=2,| +environ[2] = |COLORED_STDERR_PRIVATE_FDS=2,| Done. diff --git a/tests/lib.sh b/tests/lib.sh index 3427b56..2f2c736 100644 --- a/tests/lib.sh +++ b/tests/lib.sh @@ -40,7 +40,10 @@ fi LC_ALL=C unset LANGUAGE -# Set default COLORED_STDERR_FDS value. +# Clear user defined variables. +unset COLORED_STDERR_FDS +unset COLORED_STDERR_FORCE_WRITE +# Set default COLORED_STDERR_PRIVATE_FDS value. fds=2, @@ -78,9 +81,9 @@ run_test() { ( # Standard setup. LD_PRELOAD="$library" - COLORED_STDERR_FDS="$fds" + COLORED_STDERR_PRIVATE_FDS="$fds" export LD_PRELOAD - export COLORED_STDERR_FDS + export COLORED_STDERR_PRIVATE_FDS # Change pre/post strings for simpler testing. COLORED_STDERR_PRE='>STDERR>' diff --git a/tests/test_environment.sh b/tests/test_environment.sh index a530718..47a02a2 100755 --- a/tests/test_environment.sh +++ b/tests/test_environment.sh @@ -18,7 +18,7 @@ test "x$srcdir" = x && srcdir=. . "$srcdir/lib.sh" -# Test unexpected values for COLORED_STDERR_FDS environment variable. +# Test unexpected values for COLORED_STDERR_PRIVATE_FDS environment variable. # Empty fields. fds= @@ -44,3 +44,18 @@ test_program_subshell example example_environment fds=-20,-30,2,-1, test_program example example_environment test_program_subshell example example_environment + +# Test COLORED_STDERR_FDS overwrites COLORED_STDERR_PRIVATE_FDS. Additional +# tests in example_exec. + +fds= +COLORED_STDERR_FDS=2, +export COLORED_STDERR_FDS +test_program example example_environment +test_program_subshell example example_environment + +fds=2, +COLORED_STDERR_FDS= +export COLORED_STDERR_FDS +test_program example example_environment_empty +test_program_subshell example example_environment_empty -- 2.43.2