diff mbox series

Fix elf/tst-env-setuid[-static] if test needs to be rerun.

Message ID 20231213114450.501138-1-stli@linux.ibm.com
State New
Headers show
Series Fix elf/tst-env-setuid[-static] if test needs to be rerun. | expand

Commit Message

Stefan Liebler Dec. 13, 2023, 11:44 a.m. UTC
If /tmp is mounted nosuid and make xcheck is run,
then tst-env-setuid fails UNSUPPORTED with "SGID failed: GID and EGID match"
and /var/tmp/tst-sonamemove-runmod1.so.profile is created.

If you then try to rerun the test with a suid mounted test-dir
(the SGID binary is created in test-dir which defaults to /tmp)
with something like that:
make tst-env-setuid-ENV="TMPDIR=..." t=elf/tst-env-setuid test
the test fails as the LD_PROFILE output file is still available
from the previous run.

Thus this patch removes the LD_PROFILE output file in parent
before spawning the SGID binary.

Even if LD_PROFILE is not supported anymore in static binaries,
use a different library and thus output file for tst-env-setuid
and tst-env-setuid-static in order to not interfere if both
tests are run in parallel.

Furthermore the checks in test_child are now more verbose.
---
 elf/Makefile                |  1 +
 elf/tst-env-setuid-static.c |  1 +
 elf/tst-env-setuid.c        | 46 ++++++++++++++++++++++++++++++++-----
 3 files changed, 42 insertions(+), 6 deletions(-)

Comments

Adhemerval Zanella Netto Dec. 19, 2023, 4:48 p.m. UTC | #1
On 13/12/23 08:44, Stefan Liebler wrote:
> If /tmp is mounted nosuid and make xcheck is run,
> then tst-env-setuid fails UNSUPPORTED with "SGID failed: GID and EGID match"
> and /var/tmp/tst-sonamemove-runmod1.so.profile is created.
> 
> If you then try to rerun the test with a suid mounted test-dir
> (the SGID binary is created in test-dir which defaults to /tmp)
> with something like that:
> make tst-env-setuid-ENV="TMPDIR=..." t=elf/tst-env-setuid test
> the test fails as the LD_PROFILE output file is still available
> from the previous run.
> 
> Thus this patch removes the LD_PROFILE output file in parent
> before spawning the SGID binary.
> 
> Even if LD_PROFILE is not supported anymore in static binaries,
> use a different library and thus output file for tst-env-setuid
> and tst-env-setuid-static in order to not interfere if both
> tests are run in parallel.
> 
> Furthermore the checks in test_child are now more verbose.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  elf/Makefile                |  1 +
>  elf/tst-env-setuid-static.c |  1 +
>  elf/tst-env-setuid.c        | 46 ++++++++++++++++++++++++++++++++-----
>  3 files changed, 42 insertions(+), 6 deletions(-)
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index afec7be084..87aac923ba 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -3015,6 +3015,7 @@ tst-env-setuid-ARGS = -- $(host-test-program-cmd)
>  
>  # Reuse a module with a SONAME, to specific as the LD_PROFILE.
>  $(objpfx)tst-env-setuid: $(objpfx)tst-sonamemove-runmod2.so
> +$(objpfx)tst-env-setuid-static.out: $(objpfx)tst-sonamemove-runmod1.so
>  
>  # The object tst-nodeps1-mod.so has no explicit dependencies on libc.so.
>  $(objpfx)tst-nodeps1-mod.so: $(objpfx)tst-nodeps1-mod.os

Ok.

> diff --git a/elf/tst-env-setuid-static.c b/elf/tst-env-setuid-static.c
> index 0d88ae88b9..162d9169ec 100644
> --- a/elf/tst-env-setuid-static.c
> +++ b/elf/tst-env-setuid-static.c
> @@ -1 +1,2 @@
> +#define PROFILE_LIB      "tst-sonamemove-runmod1.so"
>  #include "tst-env-setuid.c"

Ok.

> diff --git a/elf/tst-env-setuid.c b/elf/tst-env-setuid.c
> index 9fa591a136..b4f0e547a7 100644
> --- a/elf/tst-env-setuid.c
> +++ b/elf/tst-env-setuid.c
> @@ -36,7 +36,9 @@ static char SETGID_CHILD[] = "setgid-child";
>  #define UNFILTERED_VALUE "some-unfiltered-value"
>  /* It assumes no other programs is being profile with a library with same
>     SONAME using the default folder.  */
> -#define PROFILE_LIB      "tst-sonamemove-runmod2.so"
> +#ifndef PROFILE_LIB
> +# define PROFILE_LIB      "tst-sonamemove-runmod2.so"
> +#endif
>  
>  struct envvar_t
>  {
> @@ -53,7 +55,7 @@ static const struct envvar_t filtered_envvars[] =
>    { "LD_HWCAP_MASK",           FILTERED_VALUE },
>    { "LD_LIBRARY_PATH",         FILTERED_VALUE },
>    { "LD_PRELOAD",              FILTERED_VALUE },
> -  { "LD_PROFILE",              "tst-sonamemove-runmod2.so" },
> +  { "LD_PROFILE",              PROFILE_LIB },
>    { "MALLOC_ARENA_MAX",        FILTERED_VALUE },
>    { "MALLOC_PERTURB_",         FILTERED_VALUE },
>    { "MALLOC_TRACE",            FILTERED_VALUE },
> @@ -83,7 +85,12 @@ test_child (void)
>         e++)
>      {
>        const char *env = getenv (e->env);
> -      ret |= env != NULL;
> +      if (env != NULL)
> +	{
> +	  printf ("FAIL: filtered environment variable is not NULL: %s=%s\n",
> +		  e->env, env);
> +	  ret = 1;
> +	}
>      }
>  
>    for (const struct envvar_t *e = unfiltered_envvars;
> @@ -91,13 +98,30 @@ test_child (void)
>         e++)
>      {
>        const char *env = getenv (e->env);
> -      ret |= !(env != NULL && strcmp (env, e->value) == 0);
> +      if (!(env != NULL && strcmp (env, e->value) == 0))
> +	{
> +	  if (env == NULL)
> +	    printf ("FAIL: unfiltered environment variable %s is NULL\n",
> +		    e->env);
> +	  else
> +	    printf ("FAIL: unfiltered environment variable %s=%s != %s\n",
> +		    e->env, env, e->value);
> +
> +	  ret = 1;
> +	}
>      }
>  
> -  /* Also check if no profile file was created.  */
> +  /* 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);
> -    ret |= !access (profilepath, R_OK);
> +    if (!access (profilepath, R_OK))
> +      {
> +	printf ("FAIL: LD_PROFILE file at %s was created!\n", profilepath);
> +	ret = 1;
> +      }
>      free (profilepath);
>    }
>  
> @@ -141,6 +165,16 @@ do_test (int argc, char **argv)
>  	   e++)
>  	setenv (e->env, e->value, 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);
> +      }
> +
>        int status = support_capture_subprogram_self_sgid (SETGID_CHILD);
>  
>        if (WEXITSTATUS (status) == EXIT_UNSUPPORTED)


Ok.
diff mbox series

Patch

diff --git a/elf/Makefile b/elf/Makefile
index afec7be084..87aac923ba 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -3015,6 +3015,7 @@  tst-env-setuid-ARGS = -- $(host-test-program-cmd)
 
 # Reuse a module with a SONAME, to specific as the LD_PROFILE.
 $(objpfx)tst-env-setuid: $(objpfx)tst-sonamemove-runmod2.so
+$(objpfx)tst-env-setuid-static.out: $(objpfx)tst-sonamemove-runmod1.so
 
 # The object tst-nodeps1-mod.so has no explicit dependencies on libc.so.
 $(objpfx)tst-nodeps1-mod.so: $(objpfx)tst-nodeps1-mod.os
diff --git a/elf/tst-env-setuid-static.c b/elf/tst-env-setuid-static.c
index 0d88ae88b9..162d9169ec 100644
--- a/elf/tst-env-setuid-static.c
+++ b/elf/tst-env-setuid-static.c
@@ -1 +1,2 @@ 
+#define PROFILE_LIB      "tst-sonamemove-runmod1.so"
 #include "tst-env-setuid.c"
diff --git a/elf/tst-env-setuid.c b/elf/tst-env-setuid.c
index 9fa591a136..b4f0e547a7 100644
--- a/elf/tst-env-setuid.c
+++ b/elf/tst-env-setuid.c
@@ -36,7 +36,9 @@  static char SETGID_CHILD[] = "setgid-child";
 #define UNFILTERED_VALUE "some-unfiltered-value"
 /* It assumes no other programs is being profile with a library with same
    SONAME using the default folder.  */
-#define PROFILE_LIB      "tst-sonamemove-runmod2.so"
+#ifndef PROFILE_LIB
+# define PROFILE_LIB      "tst-sonamemove-runmod2.so"
+#endif
 
 struct envvar_t
 {
@@ -53,7 +55,7 @@  static const struct envvar_t filtered_envvars[] =
   { "LD_HWCAP_MASK",           FILTERED_VALUE },
   { "LD_LIBRARY_PATH",         FILTERED_VALUE },
   { "LD_PRELOAD",              FILTERED_VALUE },
-  { "LD_PROFILE",              "tst-sonamemove-runmod2.so" },
+  { "LD_PROFILE",              PROFILE_LIB },
   { "MALLOC_ARENA_MAX",        FILTERED_VALUE },
   { "MALLOC_PERTURB_",         FILTERED_VALUE },
   { "MALLOC_TRACE",            FILTERED_VALUE },
@@ -83,7 +85,12 @@  test_child (void)
        e++)
     {
       const char *env = getenv (e->env);
-      ret |= env != NULL;
+      if (env != NULL)
+	{
+	  printf ("FAIL: filtered environment variable is not NULL: %s=%s\n",
+		  e->env, env);
+	  ret = 1;
+	}
     }
 
   for (const struct envvar_t *e = unfiltered_envvars;
@@ -91,13 +98,30 @@  test_child (void)
        e++)
     {
       const char *env = getenv (e->env);
-      ret |= !(env != NULL && strcmp (env, e->value) == 0);
+      if (!(env != NULL && strcmp (env, e->value) == 0))
+	{
+	  if (env == NULL)
+	    printf ("FAIL: unfiltered environment variable %s is NULL\n",
+		    e->env);
+	  else
+	    printf ("FAIL: unfiltered environment variable %s=%s != %s\n",
+		    e->env, env, e->value);
+
+	  ret = 1;
+	}
     }
 
-  /* Also check if no profile file was created.  */
+  /* 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);
-    ret |= !access (profilepath, R_OK);
+    if (!access (profilepath, R_OK))
+      {
+	printf ("FAIL: LD_PROFILE file at %s was created!\n", profilepath);
+	ret = 1;
+      }
     free (profilepath);
   }
 
@@ -141,6 +165,16 @@  do_test (int argc, char **argv)
 	   e++)
 	setenv (e->env, e->value, 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);
+      }
+
       int status = support_capture_subprogram_self_sgid (SETGID_CHILD);
 
       if (WEXITSTATUS (status) == EXIT_UNSUPPORTED)