diff mbox series

[3/4] Enhance setuid-tunables test

Message ID 20210316070755.330084-4-siddhesh@sourceware.org
State New
Headers show
Series tunables and setxid programs | expand

Commit Message

Siddhesh Poyarekar March 16, 2021, 7:07 a.m. UTC 
Instead of passing GLIBC_TUNABLES via the environment, pass the
environment variable from parent to child.  This allows us to test
multiple variables to ensure better coverage.

The test list currently only includes the case that's already being
tested.  More tests will be added later.
---
 elf/Makefile                  |  2 -
 elf/tst-env-setuid-tunables.c | 90 +++++++++++++++++++++++++++--------
 2 files changed, 69 insertions(+), 23 deletions(-)

Comments

Carlos O'Donell April 6, 2021, 4:46 p.m. UTC | #1 
On 3/16/21 3:07 AM, Siddhesh Poyarekar via Libc-alpha wrote:
> Instead of passing GLIBC_TUNABLES via the environment, pass the
> environment variable from parent to child.  This allows us to test
> multiple variables to ensure better coverage.
> 
> The test list currently only includes the case that's already being
> tested.  More tests will be added later.

I like that you turn this into a data-driven test and move the env vars
out of make.

LGTM.

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

> ---
>  elf/Makefile                  |  2 -
>  elf/tst-env-setuid-tunables.c | 90 +++++++++++++++++++++++++++--------
>  2 files changed, 69 insertions(+), 23 deletions(-)
> 
> diff --git a/elf/Makefile b/elf/Makefile
> index 3b8e13e066..4e04c26eea 100644
> --- a/elf/Makefile
> +++ b/elf/Makefile
> @@ -1653,8 +1653,6 @@ $(objpfx)tst-nodelete-dlclose.out: $(objpfx)tst-nodelete-dlclose-dso.so \
>  
>  tst-env-setuid-ENV = MALLOC_CHECK_=2 MALLOC_MMAP_THRESHOLD_=4096 \
>  		     LD_HWCAP_MASK=0x1
> -tst-env-setuid-tunables-ENV = \
> -	GLIBC_TUNABLES=glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096

OK.

>  
>  $(objpfx)tst-debug1: $(libdl)
>  $(objpfx)tst-debug1.out: $(objpfx)tst-debug1mod1.so
> diff --git a/elf/tst-env-setuid-tunables.c b/elf/tst-env-setuid-tunables.c
> index 50bef8683d..3d523875b1 100644
> --- a/elf/tst-env-setuid-tunables.c
> +++ b/elf/tst-env-setuid-tunables.c
> @@ -25,35 +25,50 @@
>  #include "config.h"
>  #undef _LIBC
>  
> -#define test_parent test_parent_tunables
> -#define test_child test_child_tunables
> -
> -static int test_child_tunables (void);
> -static int test_parent_tunables (void);
> -
> -#include "tst-env-setuid.c"
> +#include <errno.h>
> +#include <fcntl.h>
> +#include <stdlib.h>
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/stat.h>
> +#include <sys/wait.h>
> +#include <unistd.h>
> +#include <intprops.h>
> +#include <array_length.h>
> +
> +#include <support/check.h>
> +#include <support/support.h>
> +#include <support/test-driver.h>
> +#include <support/capture_subprocess.h>
> +
> +const char *teststrings[] =
> +{
> +  "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",

OK.

> +};
>  
> -#define CHILD_VALSTRING_VALUE "glibc.malloc.mmap_threshold=4096"
> -#define PARENT_VALSTRING_VALUE \
> -  "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096"
> +const char *resultstrings[] =
> +{
> +  "glibc.malloc.mmap_threshold=4096",

OK. SXID_IGNORE.

> +};
>  
>  static int
> -test_child_tunables (void)
> +test_child (int off)
>  {
>    const char *val = getenv ("GLIBC_TUNABLES");
>  
>  #if HAVE_TUNABLES
> -  if (val != NULL && strcmp (val, CHILD_VALSTRING_VALUE) == 0)
> +  if (val != NULL && strcmp (val, resultstrings[off]) == 0)
>      return 0;
>  
>    if (val != NULL)
> -    printf ("Unexpected GLIBC_TUNABLES VALUE %s\n", val);
> +    printf ("[%d] Unexpected GLIBC_TUNABLES VALUE %s\n", off, val);
>  
>    return 1;
>  #else
>    if (val != NULL)
>      {
> -      printf ("GLIBC_TUNABLES not cleared\n");
> +      printf ("[%d] GLIBC_TUNABLES not cleared\n", off);
>        return 1;
>      }
>    return 0;
> @@ -61,15 +76,48 @@ test_child_tunables (void)
>  }
>  
>  static int
> -test_parent_tunables (void)
> +do_test (int argc, char **argv)
>  {
> -  const char *val = getenv ("GLIBC_TUNABLES");
> +  /* Setgid child process.  */
> +  if (argc == 2)
> +    {
> +      if (getgid () == getegid ())
> +	/* This can happen if the file system is mounted nosuid.  */
> +	FAIL_UNSUPPORTED ("SGID failed: GID and EGID match (%jd)\n",
> +			  (intmax_t) getgid ());
>  
> -  if (val != NULL && strcmp (val, PARENT_VALSTRING_VALUE) == 0)
> -    return 0;
> +      int ret = test_child (atoi (argv[1]));
>  
> -  if (val != NULL)
> -    printf ("Unexpected GLIBC_TUNABLES VALUE %s\n", val);
> +      if (ret != 0)
> +	exit (1);
>  
> -  return 1;
> +      exit (EXIT_SUCCESS);
> +    }
> +  else
> +    {
> +      int ret = 0;
> +
> +      /* Spawn tests.  */
> +      for (int i = 0; i < array_length (teststrings); i++)
> +	{
> +	  char buf[INT_BUFSIZE_BOUND (int)];
> +
> +	  printf ("Spawned test for %s (%d)\n", teststrings[i], i);
> +	  snprintf (buf, sizeof (buf), "%d\n", i);
> +	  if (setenv ("GLIBC_TUNABLES", teststrings[i], 1) != 0)

OK. Put env vars into test as a data-driven test rather than Make-driven.

> +	    exit (1);
> +
> +	  int status = support_capture_subprogram_self_sgid (buf);
> +
> +	  /* Bail out early if unsupported.  */
> +	  if (WEXITSTATUS (status) == EXIT_UNSUPPORTED)
> +	    return EXIT_UNSUPPORTED;
> +
> +	  ret |= status;
> +	}
> +      return ret;
> +    }
>  }
> +
> +#define TEST_FUNCTION_ARGV do_test
> +#include <support/test-driver.c>
>
diff mbox series

Patch

diff --git a/elf/Makefile b/elf/Makefile
index 3b8e13e066..4e04c26eea 100644
--- a/elf/Makefile
+++ b/elf/Makefile
@@ -1653,8 +1653,6 @@  $(objpfx)tst-nodelete-dlclose.out: $(objpfx)tst-nodelete-dlclose-dso.so \
 
 tst-env-setuid-ENV = MALLOC_CHECK_=2 MALLOC_MMAP_THRESHOLD_=4096 \
 		     LD_HWCAP_MASK=0x1
-tst-env-setuid-tunables-ENV = \
-	GLIBC_TUNABLES=glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096
 
 $(objpfx)tst-debug1: $(libdl)
 $(objpfx)tst-debug1.out: $(objpfx)tst-debug1mod1.so
diff --git a/elf/tst-env-setuid-tunables.c b/elf/tst-env-setuid-tunables.c
index 50bef8683d..3d523875b1 100644
--- a/elf/tst-env-setuid-tunables.c
+++ b/elf/tst-env-setuid-tunables.c
@@ -25,35 +25,50 @@ 
 #include "config.h"
 #undef _LIBC
 
-#define test_parent test_parent_tunables
-#define test_child test_child_tunables
-
-static int test_child_tunables (void);
-static int test_parent_tunables (void);
-
-#include "tst-env-setuid.c"
+#include <errno.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+#include <sys/stat.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include <intprops.h>
+#include <array_length.h>
+
+#include <support/check.h>
+#include <support/support.h>
+#include <support/test-driver.h>
+#include <support/capture_subprocess.h>
+
+const char *teststrings[] =
+{
+  "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096",
+};
 
-#define CHILD_VALSTRING_VALUE "glibc.malloc.mmap_threshold=4096"
-#define PARENT_VALSTRING_VALUE \
-  "glibc.malloc.check=2:glibc.malloc.mmap_threshold=4096"
+const char *resultstrings[] =
+{
+  "glibc.malloc.mmap_threshold=4096",
+};
 
 static int
-test_child_tunables (void)
+test_child (int off)
 {
   const char *val = getenv ("GLIBC_TUNABLES");
 
 #if HAVE_TUNABLES
-  if (val != NULL && strcmp (val, CHILD_VALSTRING_VALUE) == 0)
+  if (val != NULL && strcmp (val, resultstrings[off]) == 0)
     return 0;
 
   if (val != NULL)
-    printf ("Unexpected GLIBC_TUNABLES VALUE %s\n", val);
+    printf ("[%d] Unexpected GLIBC_TUNABLES VALUE %s\n", off, val);
 
   return 1;
 #else
   if (val != NULL)
     {
-      printf ("GLIBC_TUNABLES not cleared\n");
+      printf ("[%d] GLIBC_TUNABLES not cleared\n", off);
       return 1;
     }
   return 0;
@@ -61,15 +76,48 @@  test_child_tunables (void)
 }
 
 static int
-test_parent_tunables (void)
+do_test (int argc, char **argv)
 {
-  const char *val = getenv ("GLIBC_TUNABLES");
+  /* Setgid child process.  */
+  if (argc == 2)
+    {
+      if (getgid () == getegid ())
+	/* This can happen if the file system is mounted nosuid.  */
+	FAIL_UNSUPPORTED ("SGID failed: GID and EGID match (%jd)\n",
+			  (intmax_t) getgid ());
 
-  if (val != NULL && strcmp (val, PARENT_VALSTRING_VALUE) == 0)
-    return 0;
+      int ret = test_child (atoi (argv[1]));
 
-  if (val != NULL)
-    printf ("Unexpected GLIBC_TUNABLES VALUE %s\n", val);
+      if (ret != 0)
+	exit (1);
 
-  return 1;
+      exit (EXIT_SUCCESS);
+    }
+  else
+    {
+      int ret = 0;
+
+      /* Spawn tests.  */
+      for (int i = 0; i < array_length (teststrings); i++)
+	{
+	  char buf[INT_BUFSIZE_BOUND (int)];
+
+	  printf ("Spawned test for %s (%d)\n", teststrings[i], i);
+	  snprintf (buf, sizeof (buf), "%d\n", i);
+	  if (setenv ("GLIBC_TUNABLES", teststrings[i], 1) != 0)
+	    exit (1);
+
+	  int status = support_capture_subprogram_self_sgid (buf);
+
+	  /* Bail out early if unsupported.  */
+	  if (WEXITSTATUS (status) == EXIT_UNSUPPORTED)
+	    return EXIT_UNSUPPORTED;
+
+	  ret |= status;
+	}
+      return ret;
+    }
 }
+
+#define TEST_FUNCTION_ARGV do_test
+#include <support/test-driver.c>