elf: Ignore LD_PROFILE if LD_PROFILE_OUTPUT is not set (bug 33797)

The previous default for LD_PROFILE_OUTPUT, /var/tmp, is insecure
because it's typically a 1777 directory, and other systems could
place malicious files there which interfere with execution.

Requiring the user to specify a profiling directory mitigates
the impact of bug 33797.  Clear LD_PROFILE_OUTPUT alongside
with LD_PROFILE.

Rework the test not to use predictable file names.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>

diff --git a/NEWS b/NEWS
index f5bd385bbc48a57d314eb62608e78f6a84dbe5dd..bf5167c0666f4ed2e7fc9e25a5598e1bdeb5e0de 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -100,7 +100,11 @@ Deprecated and removed features, and other changes affecting compatibility:
 
 Changes to build and runtime requirements:
 
-  [Add changes to build and runtime requirements here]
+* The LD_PROFILE functionality no longer has a default directory for the
+  profile data it writes.  Instead, developers are required to set a
+  directory explicitly using the LD_PROFILE_OUTPUT environment variable.
+  To restore the previous, insecure behavior, processes can be run with
+  LD_PROFILE_OUTPUT=/var/tmp.
 
 Security related changes:
 

diff --git a/elf/rtld.c b/elf/rtld.c
index 9842f31ee47ae85170d5a64f92cce10c5d50e32f..29e7a4ddfa49e100f8bb1b7632bfa18d0e79cfa5 100644 (file)
--- a/elf/rtld.c
+++ b/elf/rtld.c
@@ -359,7 +359,6 @@ struct rtld_global_ro _rtld_global_ro attribute_relro =
     ._dl_fpu_control = _FPU_DEFAULT,
     ._dl_pagesize = EXEC_PAGESIZE,
     ._dl_inhibit_cache = 0,
-    ._dl_profile_output = "/var/tmp",
 
     /* Function pointers.  */
     ._dl_debug_printf = _dl_debug_printf,
@@ -2708,6 +2707,15 @@ process_envvars_default (struct dl_main_state *state)
        }
     }
 
+  /* There is no fixed, safe directory to store profiling data, so
+     activate LD_PROFILE only if LD_PROFILE_OUTPUT is set as well.  */
+  if (GLRO(dl_profile) != NULL && GLRO(dl_profile_output) == NULL)
+    {
+      _dl_error_printf ("\
+warning: LD_PROFILE ignored because LD_PROFILE_OUTPUT not specified\n");
+      GLRO(dl_profile) = NULL;
+    }
+
   /* If we have to run the dynamic linker in debugging mode and the
      LD_DEBUG_OUTPUT environment variable is given, we write the debug
      messages to this file.  */

diff --git a/elf/tst-env-setuid.c b/elf/tst-env-setuid.c
index 830f330a371bb53573a8434eec46b11c7b3cda98..ef8b968004f25c1abd99a048a97f1c1d15139862 100644 (file)
--- a/elf/tst-env-setuid.c
+++ b/elf/tst-env-setuid.c
@@ -40,7 +40,11 @@ static char SETGID_CHILD[] = "setgid-child";
 # define PROFILE_LIB      "tst-sonamemove-runmod2.so"
 #endif
 
-#define LD_DEBUG_OUTPUT   "/tmp/some-file"
+/* Computed path for LD_DEBUG_OUTPUT.  */
+static char *debugoutputpath;
+
+/* Expected file name for erroneous LD_PROFILE output.  */
+static char *profilepath;
 
 struct envvar_t
 {
@@ -57,13 +61,14 @@ static const struct envvar_t filtered_envvars[] =
   { "LD_LIBRARY_PATH",         FILTERED_VALUE },
   { "LD_PRELOAD",              FILTERED_VALUE },
   { "LD_PROFILE",              PROFILE_LIB },
+  { "LD_PROFILE_OUTPUT",       "/var/tmp" },      /* Not actually used.  */
   { "MALLOC_ARENA_MAX",        FILTERED_VALUE },
   { "MALLOC_PERTURB_",         FILTERED_VALUE },
   { "MALLOC_TRACE",            FILTERED_VALUE },
   { "MALLOC_TRIM_THRESHOLD_",  FILTERED_VALUE },
   { "RES_OPTIONS",             FILTERED_VALUE },
   { "LD_DEBUG",                "all" },
-  { "LD_DEBUG_OUTPUT",         LD_DEBUG_OUTPUT },
+  { "LD_DEBUG_OUTPUT",         "overwritten" },   /* Not actually used.  */
   { "LD_WARN",                 FILTERED_VALUE },
   { "LD_VERBOSE",              FILTERED_VALUE },
   { "LD_BIND_NOW",             "0" },
@@ -79,7 +84,7 @@ static const struct envvar_t unfiltered_envvars[] =
 static void
 unlink_ld_debug_output (pid_t pid)
 {
-  char *output = xasprintf ("%s.%d", LD_DEBUG_OUTPUT, pid);
+  char *output = xasprintf ("%s.%d", debugoutputpath, pid);
   unlink (output);
   free (output);
 }
@@ -121,18 +126,12 @@ test_child (void)
     }
 
   /* Also check if no profile file was created.
-     The parent sets LD_DEBUG_OUTPUT="/tmp/some-file"
-     which should be filtered.  Then it falls back to "/var/tmp".
      Note: LD_PROFILE is not supported for static binaries.  */
-  {
-    char *profilepath = xasprintf ("/var/tmp/%s.profile", PROFILE_LIB);
-    if (!access (profilepath, R_OK))
-      {
-       printf ("FAIL: LD_PROFILE file at %s was created!\n", profilepath);
-       ret = 1;
-      }
-    free (profilepath);
-  }
+  if (!access (profilepath, R_OK))
+    {
+      printf ("FAIL: LD_PROFILE file at %s was created!\n", profilepath);
+      ret = 1;
+    }
 
   return ret;
 }
@@ -145,6 +144,11 @@ do_test (int argc, char **argv)
   if (argc >= 2 && strstr (argv[1], LD_SO) != 0)
     FAIL_UNSUPPORTED ("dynamic test requires --enable-hardcoded-path-in-tests");
 
+  profilepath = xasprintf ("%s/%s.profile",
+                          support_objdir_root, PROFILE_LIB);
+  debugoutputpath = xasprintf ("%s/tst-env-setuid-file",
+                              support_objdir_root);
+
   /* Setgid child process.  */
   if (argc == 2 && strcmp (argv[1], SETGID_CHILD) == 0)
     {
@@ -165,7 +169,6 @@ do_test (int argc, char **argv)
 
       if (ret != 0)
        exit (1);
-      return 0;
     }
   else
     {
@@ -179,20 +182,25 @@ do_test (int argc, char **argv)
           e++)
        setenv (e->env, e->value, 1);
 
+      /* Dynamically computed values.  */
+      setenv ("LD_DEBUG_OUTPUT", debugoutputpath, 1);
+      setenv ("LD_PROFILE_OUTPUT", support_objdir_root, 1);
+
       /* Ensure that the profile output does not exist from a previous run
         (e.g. if test_dir, which defaults to /tmp, is mounted nosuid.)
         Note: support_capture_subprogram_self_sgid creates the SGID binary
         in test_dir.  */
-      {
-       char *profilepath = xasprintf ("/var/tmp/%s.profile", PROFILE_LIB);
-       unlink (profilepath);
-       free (profilepath);
-      }
+      unlink (profilepath);
 
       support_capture_subprogram_self_sgid (SETGID_CHILD);
 
-      return 0;
+      /* And clean up afterwards if necessary.  */
+      unlink (profilepath);
     }
+
+  free (profilepath);
+  free (debugoutputpath);
+  return 0;
 }
 
 #define TEST_FUNCTION_ARGV do_test

diff --git a/sysdeps/generic/unsecvars.h b/sysdeps/generic/unsecvars.h
index 97857a11aaafff4553d963da4ef9753e612dce02..33755179f348dd3263c5469f7f0320bbbbea60e5 100644 (file)
--- a/sysdeps/generic/unsecvars.h
+++ b/sysdeps/generic/unsecvars.h
@@ -16,6 +16,7 @@
   "LD_ORIGIN_PATH\0"                                                         \
   "LD_PRELOAD\0"                                                             \
   "LD_PROFILE\0"                                                             \
+  "LD_PROFILE_OUTPUT\0"                                                              \
   "LD_SHOW_AUXV\0"                                                           \
   "LD_VERBOSE\0"                                                             \
   "LD_WARN\0"                                                                \