public inbox for libc-alpha@sourceware.org
 help / color / mirror / Atom feed
* [PATCH] wordexp: Split out command execution tests from posix/wordexp-test
@ 2019-10-08 11:04 Florian Weimer
  2019-10-08 13:40 ` Adhemerval Zanella
  2019-10-08 15:58 ` Joseph Myers
  0 siblings, 2 replies; 4+ messages in thread
From: Florian Weimer @ 2019-10-08 11:04 UTC (permalink / raw)
  To: libc-alpha

Once wordexp switches to posix_spawn, testing for command execution
based on fork handlers will not work anymore.  Therefore, move these
subtests into  a new test, posix/tst-wordexp-nocmd, which uses a
different form of command execution detection, based on PID
namespaces.

2019-10-08  Florian Weimer  <fweimer@redhat.com>

	* posix/Makefile (tests): Add tst-wordexp-nocmd.
	* posix/wordexp-test.c (__app_register_atfork): Remove function.
	(registered_forks): Remove variable.
	(register_fork): Remove function.
	(test_case): Remove WRDE_NOCMD tests.
	(main): Do not set up fork tracking.  Remove integer overflow in
	division tests.
	(testit): Do not check for unexpected forks.
	* posix/tst-wordexp-nocmd.c: New file containing the tests removed
	from posix/wordexp-test.c.

diff --git a/posix/Makefile b/posix/Makefile
index 0579596d9e..fe031eb450 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -100,7 +100,8 @@ tests		:= test-errno tstgetopt testfnm runtests runptests \
 		   tst-posix_fadvise tst-posix_fadvise64 \
 		   tst-sysconf-empty-chroot tst-glob_symlinks tst-fexecve \
 		   tst-glob-tilde test-ssize-max tst-spawn4 bug-regex37 \
-		   bug-regex38 tst-regcomp-truncated tst-spawn-chdir
+		   bug-regex38 tst-regcomp-truncated tst-spawn-chdir \
+		   tst-wordexp-nocmd
 tests-internal	:= bug-regex5 bug-regex20 bug-regex33 \
 		   tst-rfc3484 tst-rfc3484-2 tst-rfc3484-3 \
 		   tst-glob_lstat_compat tst-spawn4-compat
diff --git a/posix/tst-wordexp-nocmd.c b/posix/tst-wordexp-nocmd.c
new file mode 100644
index 0000000000..b2f64c819f
--- /dev/null
+++ b/posix/tst-wordexp-nocmd.c
@@ -0,0 +1,179 @@
+/* Test for (lack of) command execution in wordexp.
+   Copyright (C) 1997-2019 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+/* This test optionally counts PIDs in a PID namespace to detect
+   forks.  Without kernel support for that, it will merely look at the
+   error codes from wordexp to check that no command execution
+   happens.  */
+
+#include <sched.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <support/check.h>
+#include <support/namespace.h>
+#include <support/xunistd.h>
+#include <wordexp.h>
+
+/* Set to true if the test runs in a PID namespace and can therefore
+   use next_pid below.  */
+static bool pid_tests_supported;
+
+/* The next PID, as returned from next_pid below.  Only meaningful if
+   pid_tests_supported.  */
+static pid_t expected_pid;
+
+/* Allocate the next PID and return it.  The process is terminated.
+   Note that the test itself advances the next PID.  */
+static pid_t
+next_pid (void)
+{
+  pid_t pid = xfork ();
+  if (pid == 0)
+    _exit (0);
+  xwaitpid (pid, NULL, 0);
+  return pid;
+}
+
+/* Check that evaluating PATTERN with WRDE_NOCMD results in
+   EXPECTED_ERROR.  */
+static void
+expect_failure (const char *pattern, int expected_error)
+{
+  printf ("info: testing pattern: %s\n", pattern);
+  wordexp_t w;
+  TEST_COMPARE (wordexp (pattern, &w, WRDE_NOCMD), expected_error);
+  if (pid_tests_supported)
+    TEST_COMPARE (expected_pid++, next_pid ());
+}
+
+/* Run all the tests.  Invoked with different IFS values.  */
+static void
+run_tests (void)
+{
+  /* Integer overflow in division.  */
+  {
+    static const char *const numbers[] = {
+      "0",
+      "1",
+      "65536",
+      "2147483648",
+      "4294967296"
+      "9223372036854775808",
+      "18446744073709551616",
+      "170141183460469231731687303715884105728",
+      "340282366920938463463374607431768211456",
+      NULL
+    };
+
+    for (const char *const *num = numbers; *num != NULL; ++num)
+      {
+        wordexp_t w;
+        char pattern[256];
+        snprintf (pattern, sizeof (pattern), "$[(-%s)/(-1)]", *num);
+        int ret = wordexp (pattern, &w, WRDE_NOCMD);
+        if (ret == 0)
+          {
+            /* If the call is successful, the result must match the
+               original number.  */
+            TEST_COMPARE (w.we_wordc, 1);
+            TEST_COMPARE_STRING (w.we_wordv[0], *num);
+            TEST_COMPARE_STRING (w.we_wordv[1], NULL);
+            wordfree (&w);
+          }
+        else
+          /* Otherwise, the test must fail with a syntax error.  */
+          TEST_COMPARE (ret, WRDE_SYNTAX);
+
+        /* In both cases, command execution is not permitted.  */
+        if (pid_tests_supported)
+          TEST_COMPARE (expected_pid++, next_pid ());
+      }
+  }
+
+  /* (Lack of) command execution tests.  */
+
+  expect_failure ("$(ls)", WRDE_CMDSUB);
+
+  /* Test for CVE-2014-7817. We test 3 combinations of command
+     substitution inside an arithmetic expression to make sure that
+     no commands are executed and error is returned.  */
+  expect_failure ("$((`echo 1`))", WRDE_CMDSUB);
+  expect_failure ("$((1+`echo 1`))", WRDE_CMDSUB);
+  expect_failure ("$((1+$((`echo 1`))))", WRDE_CMDSUB);
+
+  expect_failure ("$[1/0]", WRDE_SYNTAX); /* BZ 18100.  */
+}
+
+static void
+subprocess (void *closure)
+{
+  expected_pid = 2;
+  if (pid_tests_supported)
+    TEST_COMPARE (expected_pid++, next_pid ());
+
+  /* Check that triggering command execution via wordexp results in a
+     PID increase.  */
+  if (pid_tests_supported)
+    {
+      wordexp_t w;
+      TEST_COMPARE (wordexp ("$(echo Test)", &w, 0), 0);
+      TEST_COMPARE (w.we_wordc, 1);
+      TEST_COMPARE_STRING (w.we_wordv[0], "Test");
+      TEST_COMPARE_STRING (w.we_wordv[1], NULL);
+      wordfree (&w);
+
+      pid_t n = next_pid ();
+      printf ("info: self-test resulted in PID %d (processes created: %d)\n",
+              (int) n, (int) (n - expected_pid));
+      TEST_VERIFY (n > expected_pid);
+      expected_pid = n + 1;
+  }
+
+  puts ("info: testing without IFS");
+  unsetenv ("IFS");
+  run_tests ();
+
+  puts ("info: testing with IFS");
+  TEST_COMPARE (setenv ("IFS", " \t\n", 1), 0);
+  run_tests ();
+}
+
+static int
+do_test (void)
+{
+  support_become_root ();
+
+#ifdef CLONE_NEWPID
+  if (unshare (CLONE_NEWPID) != 0)
+    printf ("warning: unshare (CLONE_NEW_PID) failed: %m\n"
+            "warning: This leads to reduced test coverage.\n");
+  else
+    pid_tests_supported = true;
+#else
+  printf ("warning: CLONE_NEW_PID not available.\n"
+          "warning: This leads to reduced test coverage.\n");
+#endif
+
+  /* CLONE_NEWPID only has an effect after fork.  */
+  support_isolate_in_subprocess (subprocess, NULL);
+
+  return 0;
+}
+
+#include <support/test-driver.c>
diff --git a/posix/wordexp-test.c b/posix/wordexp-test.c
index 957184cf47..a4d8bcf1da 100644
--- a/posix/wordexp-test.c
+++ b/posix/wordexp-test.c
@@ -31,23 +31,6 @@
 
 #define IFS " \n\t"
 
-extern int __register_atfork (void (*) (void), void (*) (void), void (*) (void), void *);
-
-static int __app_register_atfork (void (*prepare) (void), void (*parent) (void), void (*child) (void))
-{
-  return __register_atfork (prepare, parent, child, __dso_handle);
-}
-
-/* Number of forks seen.  */
-static int registered_forks;
-
-/* For each fork increment the fork count.  */
-static void
-register_fork (void)
-{
-  registered_forks++;
-}
-
 struct test_case_struct
 {
   int retval;
@@ -217,7 +200,6 @@ struct test_case_struct
     { WRDE_BADCHAR, NULL, "close-paren)", 0, 0, { NULL, }, IFS },
     { WRDE_BADCHAR, NULL, "{open-brace", 0, 0, { NULL, }, IFS },
     { WRDE_BADCHAR, NULL, "close-brace}", 0, 0, { NULL, }, IFS },
-    { WRDE_CMDSUB, NULL, "$(ls)", WRDE_NOCMD, 0, { NULL, }, IFS },
     { WRDE_BADVAL, NULL, "$var", WRDE_UNDEF, 0, { NULL, }, IFS },
     { WRDE_BADVAL, NULL, "$9", WRDE_UNDEF, 0, { NULL, }, IFS },
     { WRDE_SYNTAX, NULL, "$[50+20))", 0, 0, { NULL, }, IFS },
@@ -227,17 +209,10 @@ struct test_case_struct
     { WRDE_SYNTAX, NULL, "$((2+))", 0, 0, { NULL, }, IFS },
     { WRDE_SYNTAX, NULL, "`", 0, 0, { NULL, }, IFS },
     { WRDE_SYNTAX, NULL, "$((010+4+))", 0, 0, { NULL }, IFS },
-    /* Test for CVE-2014-7817. We test 3 combinations of command
-       substitution inside an arithmetic expression to make sure that
-       no commands are executed and error is returned.  */
-    { WRDE_CMDSUB, NULL, "$((`echo 1`))", WRDE_NOCMD, 0, { NULL, }, IFS },
-    { WRDE_CMDSUB, NULL, "$((1+`echo 1`))", WRDE_NOCMD, 0, { NULL, }, IFS },
-    { WRDE_CMDSUB, NULL, "$((1+$((`echo 1`))))", WRDE_NOCMD, 0, { NULL, }, IFS },
 
     { WRDE_SYNTAX, NULL, "`\\", 0, 0, { NULL, }, IFS },     /* BZ 18042  */
     { WRDE_SYNTAX, NULL, "${", 0, 0, { NULL, }, IFS },      /* BZ 18043  */
     { WRDE_SYNTAX, NULL, "L${a:", 0, 0, { NULL, }, IFS },   /* BZ 18043#c4  */
-    { WRDE_SYNTAX, NULL, "$[1/0]", WRDE_NOCMD, 0, {NULL, }, IFS }, /* BZ 18100 */
 
     { -1, NULL, NULL, 0, 0, { NULL, }, IFS },
   };
@@ -290,15 +265,6 @@ main (int argc, char *argv[])
 	  return -1;
     }
 
-  /* If we are not allowed to do command substitution, we install
-     fork handlers to verify that no forks happened.  No forks should
-     happen at all if command substitution is disabled.  */
-  if (__app_register_atfork (register_fork, NULL, NULL) != 0)
-    {
-      printf ("Failed to register fork handler.\n");
-      return -1;
-    }
-
   for (test = 0; test_case[test].retval != -1; test++)
     if (testit (&test_case[test]))
       ++fail;
@@ -363,45 +329,6 @@ main (int argc, char *argv[])
 	++fail;
     }
 
-  /* Integer overflow in division.  */
-  {
-    static const char *const numbers[] = {
-      "0",
-      "1",
-      "65536",
-      "2147483648",
-      "4294967296"
-      "9223372036854775808",
-      "18446744073709551616",
-      "170141183460469231731687303715884105728",
-      "340282366920938463463374607431768211456",
-      NULL
-    };
-
-    for (const char *const *num = numbers; *num; ++num)
-      {
-	wordexp_t p;
-	char pattern[256];
-	snprintf (pattern, sizeof (pattern), "$[(-%s)/(-1)]", *num);
-	int ret = wordexp (pattern, &p, WRDE_NOCMD);
-	if (ret == 0)
-	  {
-	    if (p.we_wordc != 1 || strcmp (p.we_wordv[0], *num) != 0)
-	      {
-		printf ("Integer overflow for \"%s\" failed", pattern);
-		++fail;
-	      }
-	    wordfree (&p);
-	  }
-	else if (ret != WRDE_SYNTAX)
-	  {
-	    printf ("Integer overflow for \"%s\" failed with %d",
-		    pattern, ret);
-	    ++fail;
-	  }
-      }
-  }
-
   puts ("tests completed, now cleaning up");
 
   /* Clean up */
@@ -472,9 +399,6 @@ testit (struct test_case_struct *tc)
   fflush (NULL);
   const char *words = at_page_end (tc->words);
 
-  if (tc->flags & WRDE_NOCMD)
-    registered_forks = 0;
-
   if (tc->flags & WRDE_APPEND)
     {
       /* initial wordexp() call, to be appended to */
@@ -486,13 +410,6 @@ testit (struct test_case_struct *tc)
     }
   retval = wordexp (words, &we, tc->flags);
 
-  if ((tc->flags & WRDE_NOCMD)
-      && (registered_forks > 0))
-    {
-	  printf ("FAILED fork called for WRDE_NOCMD\n");
-	  return 1;
-    }
-
   if (tc->flags & WRDE_DOOFFS)
       start_offs = sav_we.we_offs;
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] wordexp: Split out command execution tests from posix/wordexp-test
  2019-10-08 11:04 [PATCH] wordexp: Split out command execution tests from posix/wordexp-test Florian Weimer
@ 2019-10-08 13:40 ` Adhemerval Zanella
  2019-10-08 15:58 ` Joseph Myers
  1 sibling, 0 replies; 4+ messages in thread
From: Adhemerval Zanella @ 2019-10-08 13:40 UTC (permalink / raw)
  To: libc-alpha



On 08/10/2019 08:04, Florian Weimer wrote:
> Once wordexp switches to posix_spawn, testing for command execution
> based on fork handlers will not work anymore.  Therefore, move these
> subtests into  a new test, posix/tst-wordexp-nocmd, which uses a
> different form of command execution detection, based on PID
> namespaces.

I was exploring using ptrace to trap on PTRACE_O_TRACECLONE, it has the
slight advantage of work on older kernels than 3.8 (CLONE_NEWPID) and
not require CAP_SYS_ADMIN (although we would need to handle YAMA). I
don't have a strong opinion, assuming pid allocation will done always 
sequentially (as explicit done by core-api/idr in Linux), this method 
should be ok.

> 
> 2019-10-08  Florian Weimer  <fweimer@redhat.com>
> 
> 	* posix/Makefile (tests): Add tst-wordexp-nocmd.
> 	* posix/wordexp-test.c (__app_register_atfork): Remove function.
> 	(registered_forks): Remove variable.
> 	(register_fork): Remove function.
> 	(test_case): Remove WRDE_NOCMD tests.
> 	(main): Do not set up fork tracking.  Remove integer overflow in
> 	division tests.
> 	(testit): Do not check for unexpected forks.
> 	* posix/tst-wordexp-nocmd.c: New file containing the tests removed
> 	from posix/wordexp-test.c.

Ok.

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

> 
> diff --git a/posix/Makefile b/posix/Makefile
> index 0579596d9e..fe031eb450 100644
> --- a/posix/Makefile
> +++ b/posix/Makefile
> @@ -100,7 +100,8 @@ tests		:= test-errno tstgetopt testfnm runtests runptests \
>  		   tst-posix_fadvise tst-posix_fadvise64 \
>  		   tst-sysconf-empty-chroot tst-glob_symlinks tst-fexecve \
>  		   tst-glob-tilde test-ssize-max tst-spawn4 bug-regex37 \
> -		   bug-regex38 tst-regcomp-truncated tst-spawn-chdir
> +		   bug-regex38 tst-regcomp-truncated tst-spawn-chdir \
> +		   tst-wordexp-nocmd
>  tests-internal	:= bug-regex5 bug-regex20 bug-regex33 \
>  		   tst-rfc3484 tst-rfc3484-2 tst-rfc3484-3 \
>  		   tst-glob_lstat_compat tst-spawn4-compat

OK.

> diff --git a/posix/tst-wordexp-nocmd.c b/posix/tst-wordexp-nocmd.c
> new file mode 100644
> index 0000000000..b2f64c819f
> --- /dev/null
> +++ b/posix/tst-wordexp-nocmd.c
> @@ -0,0 +1,179 @@
> +/* Test for (lack of) command execution in wordexp.
> +   Copyright (C) 1997-2019 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +/* This test optionally counts PIDs in a PID namespace to detect
> +   forks.  Without kernel support for that, it will merely look at the
> +   error codes from wordexp to check that no command execution
> +   happens.  */
> +
> +#include <sched.h>
> +#include <stdbool.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <support/check.h>
> +#include <support/namespace.h>
> +#include <support/xunistd.h>
> +#include <wordexp.h>
> +
> +/* Set to true if the test runs in a PID namespace and can therefore
> +   use next_pid below.  */
> +static bool pid_tests_supported;
> +
> +/* The next PID, as returned from next_pid below.  Only meaningful if
> +   pid_tests_supported.  */
> +static pid_t expected_pid;
> +
> +/* Allocate the next PID and return it.  The process is terminated.
> +   Note that the test itself advances the next PID.  */
> +static pid_t
> +next_pid (void)
> +{
> +  pid_t pid = xfork ();
> +  if (pid == 0)
> +    _exit (0);
> +  xwaitpid (pid, NULL, 0);
> +  return pid;
> +}
> +
> +/* Check that evaluating PATTERN with WRDE_NOCMD results in
> +   EXPECTED_ERROR.  */
> +static void
> +expect_failure (const char *pattern, int expected_error)
> +{
> +  printf ("info: testing pattern: %s\n", pattern);
> +  wordexp_t w;
> +  TEST_COMPARE (wordexp (pattern, &w, WRDE_NOCMD), expected_error);
> +  if (pid_tests_supported)
> +    TEST_COMPARE (expected_pid++, next_pid ());
> +}
> +

Ok.

> +/* Run all the tests.  Invoked with different IFS values.  */
> +static void
> +run_tests (void)
> +{
> +  /* Integer overflow in division.  */
> +  {
> +    static const char *const numbers[] = {
> +      "0",
> +      "1",
> +      "65536",
> +      "2147483648",
> +      "4294967296"
> +      "9223372036854775808",
> +      "18446744073709551616",
> +      "170141183460469231731687303715884105728",
> +      "340282366920938463463374607431768211456",
> +      NULL
> +    };
> +
> +    for (const char *const *num = numbers; *num != NULL; ++num)
> +      {
> +        wordexp_t w;
> +        char pattern[256];
> +        snprintf (pattern, sizeof (pattern), "$[(-%s)/(-1)]", *num);
> +        int ret = wordexp (pattern, &w, WRDE_NOCMD);
> +        if (ret == 0)
> +          {
> +            /* If the call is successful, the result must match the
> +               original number.  */
> +            TEST_COMPARE (w.we_wordc, 1);
> +            TEST_COMPARE_STRING (w.we_wordv[0], *num);
> +            TEST_COMPARE_STRING (w.we_wordv[1], NULL);
> +            wordfree (&w);
> +          }
> +        else
> +          /* Otherwise, the test must fail with a syntax error.  */
> +          TEST_COMPARE (ret, WRDE_SYNTAX);
> +
> +        /* In both cases, command execution is not permitted.  */
> +        if (pid_tests_supported)
> +          TEST_COMPARE (expected_pid++, next_pid ());
> +      }
> +  }

Ok.

> +
> +  /* (Lack of) command execution tests.  */
> +
> +  expect_failure ("$(ls)", WRDE_CMDSUB);
> +
> +  /* Test for CVE-2014-7817. We test 3 combinations of command
> +     substitution inside an arithmetic expression to make sure that
> +     no commands are executed and error is returned.  */
> +  expect_failure ("$((`echo 1`))", WRDE_CMDSUB);
> +  expect_failure ("$((1+`echo 1`))", WRDE_CMDSUB);
> +  expect_failure ("$((1+$((`echo 1`))))", WRDE_CMDSUB);
> +
> +  expect_failure ("$[1/0]", WRDE_SYNTAX); /* BZ 18100.  */
> +}
> +
> +static void
> +subprocess (void *closure)
> +{
> +  expected_pid = 2;
> +  if (pid_tests_supported)
> +    TEST_COMPARE (expected_pid++, next_pid ());
> +
> +  /* Check that triggering command execution via wordexp results in a
> +     PID increase.  */
> +  if (pid_tests_supported)
> +    {
> +      wordexp_t w;
> +      TEST_COMPARE (wordexp ("$(echo Test)", &w, 0), 0);
> +      TEST_COMPARE (w.we_wordc, 1);
> +      TEST_COMPARE_STRING (w.we_wordv[0], "Test");
> +      TEST_COMPARE_STRING (w.we_wordv[1], NULL);
> +      wordfree (&w);
> +
> +      pid_t n = next_pid ();
> +      printf ("info: self-test resulted in PID %d (processes created: %d)\n",
> +              (int) n, (int) (n - expected_pid));
> +      TEST_VERIFY (n > expected_pid);
> +      expected_pid = n + 1;
> +  }
> +
> +  puts ("info: testing without IFS");
> +  unsetenv ("IFS");
> +  run_tests ();
> +
> +  puts ("info: testing with IFS");
> +  TEST_COMPARE (setenv ("IFS", " \t\n", 1), 0);
> +  run_tests ();
> +}

Ok.

> +
> +static int
> +do_test (void)
> +{
> +  support_become_root ();
> +
> +#ifdef CLONE_NEWPID
> +  if (unshare (CLONE_NEWPID) != 0)
> +    printf ("warning: unshare (CLONE_NEW_PID) failed: %m\n"
> +            "warning: This leads to reduced test coverage.\n");
> +  else
> +    pid_tests_supported = true;
> +#else
> +  printf ("warning: CLONE_NEW_PID not available.\n"
> +          "warning: This leads to reduced test coverage.\n");
> +#endif
> +
> +  /* CLONE_NEWPID only has an effect after fork.  */
> +  support_isolate_in_subprocess (subprocess, NULL);
> +
> +  return 0;
> +}
> +
> +#include <support/test-driver.c>

Ok.

> diff --git a/posix/wordexp-test.c b/posix/wordexp-test.c
> index 957184cf47..a4d8bcf1da 100644
> --- a/posix/wordexp-test.c
> +++ b/posix/wordexp-test.c
> @@ -31,23 +31,6 @@
>  
>  #define IFS " \n\t"
>  
> -extern int __register_atfork (void (*) (void), void (*) (void), void (*) (void), void *);
> -
> -static int __app_register_atfork (void (*prepare) (void), void (*parent) (void), void (*child) (void))
> -{
> -  return __register_atfork (prepare, parent, child, __dso_handle);
> -}
> -
> -/* Number of forks seen.  */
> -static int registered_forks;
> -
> -/* For each fork increment the fork count.  */
> -static void
> -register_fork (void)
> -{
> -  registered_forks++;
> -}
> -
>  struct test_case_struct
>  {
>    int retval;

Ok.

> @@ -217,7 +200,6 @@ struct test_case_struct
>      { WRDE_BADCHAR, NULL, "close-paren)", 0, 0, { NULL, }, IFS },
>      { WRDE_BADCHAR, NULL, "{open-brace", 0, 0, { NULL, }, IFS },
>      { WRDE_BADCHAR, NULL, "close-brace}", 0, 0, { NULL, }, IFS },
> -    { WRDE_CMDSUB, NULL, "$(ls)", WRDE_NOCMD, 0, { NULL, }, IFS },
>      { WRDE_BADVAL, NULL, "$var", WRDE_UNDEF, 0, { NULL, }, IFS },
>      { WRDE_BADVAL, NULL, "$9", WRDE_UNDEF, 0, { NULL, }, IFS },
>      { WRDE_SYNTAX, NULL, "$[50+20))", 0, 0, { NULL, }, IFS },

Ok.

> @@ -227,17 +209,10 @@ struct test_case_struct
>      { WRDE_SYNTAX, NULL, "$((2+))", 0, 0, { NULL, }, IFS },
>      { WRDE_SYNTAX, NULL, "`", 0, 0, { NULL, }, IFS },
>      { WRDE_SYNTAX, NULL, "$((010+4+))", 0, 0, { NULL }, IFS },
> -    /* Test for CVE-2014-7817. We test 3 combinations of command
> -       substitution inside an arithmetic expression to make sure that
> -       no commands are executed and error is returned.  */
> -    { WRDE_CMDSUB, NULL, "$((`echo 1`))", WRDE_NOCMD, 0, { NULL, }, IFS },
> -    { WRDE_CMDSUB, NULL, "$((1+`echo 1`))", WRDE_NOCMD, 0, { NULL, }, IFS },
> -    { WRDE_CMDSUB, NULL, "$((1+$((`echo 1`))))", WRDE_NOCMD, 0, { NULL, }, IFS },
>  
>      { WRDE_SYNTAX, NULL, "`\\", 0, 0, { NULL, }, IFS },     /* BZ 18042  */
>      { WRDE_SYNTAX, NULL, "${", 0, 0, { NULL, }, IFS },      /* BZ 18043  */
>      { WRDE_SYNTAX, NULL, "L${a:", 0, 0, { NULL, }, IFS },   /* BZ 18043#c4  */
> -    { WRDE_SYNTAX, NULL, "$[1/0]", WRDE_NOCMD, 0, {NULL, }, IFS }, /* BZ 18100 */
>  
>      { -1, NULL, NULL, 0, 0, { NULL, }, IFS },
>    };

Ok.

> @@ -290,15 +265,6 @@ main (int argc, char *argv[])
>  	  return -1;
>      }
>  
> -  /* If we are not allowed to do command substitution, we install
> -     fork handlers to verify that no forks happened.  No forks should
> -     happen at all if command substitution is disabled.  */
> -  if (__app_register_atfork (register_fork, NULL, NULL) != 0)
> -    {
> -      printf ("Failed to register fork handler.\n");
> -      return -1;
> -    }
> -
>    for (test = 0; test_case[test].retval != -1; test++)
>      if (testit (&test_case[test]))
>        ++fail;

Ok.

> @@ -363,45 +329,6 @@ main (int argc, char *argv[])
>  	++fail;
>      }
>  
> -  /* Integer overflow in division.  */
> -  {
> -    static const char *const numbers[] = {
> -      "0",
> -      "1",
> -      "65536",
> -      "2147483648",
> -      "4294967296"
> -      "9223372036854775808",
> -      "18446744073709551616",
> -      "170141183460469231731687303715884105728",
> -      "340282366920938463463374607431768211456",
> -      NULL
> -    };
> -
> -    for (const char *const *num = numbers; *num; ++num)
> -      {
> -	wordexp_t p;
> -	char pattern[256];
> -	snprintf (pattern, sizeof (pattern), "$[(-%s)/(-1)]", *num);
> -	int ret = wordexp (pattern, &p, WRDE_NOCMD);
> -	if (ret == 0)
> -	  {
> -	    if (p.we_wordc != 1 || strcmp (p.we_wordv[0], *num) != 0)
> -	      {
> -		printf ("Integer overflow for \"%s\" failed", pattern);
> -		++fail;
> -	      }
> -	    wordfree (&p);
> -	  }
> -	else if (ret != WRDE_SYNTAX)
> -	  {
> -	    printf ("Integer overflow for \"%s\" failed with %d",
> -		    pattern, ret);
> -	    ++fail;
> -	  }
> -      }
> -  }
> -
>    puts ("tests completed, now cleaning up");
>  
>    /* Clean up */

Ok.

> @@ -472,9 +399,6 @@ testit (struct test_case_struct *tc)
>    fflush (NULL);
>    const char *words = at_page_end (tc->words);
>  
> -  if (tc->flags & WRDE_NOCMD)
> -    registered_forks = 0;
> -
>    if (tc->flags & WRDE_APPEND)
>      {
>        /* initial wordexp() call, to be appended to */
> @@ -486,13 +410,6 @@ testit (struct test_case_struct *tc)
>      }
>    retval = wordexp (words, &we, tc->flags);
>  
> -  if ((tc->flags & WRDE_NOCMD)
> -      && (registered_forks > 0))
> -    {
> -	  printf ("FAILED fork called for WRDE_NOCMD\n");
> -	  return 1;
> -    }
> -
>    if (tc->flags & WRDE_DOOFFS)
>        start_offs = sav_we.we_offs;
>  
> 

Ok.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] wordexp: Split out command execution tests from posix/wordexp-test
  2019-10-08 11:04 [PATCH] wordexp: Split out command execution tests from posix/wordexp-test Florian Weimer
  2019-10-08 13:40 ` Adhemerval Zanella
@ 2019-10-08 15:58 ` Joseph Myers
  2019-10-08 17:00   ` Florian Weimer
  1 sibling, 1 reply; 4+ messages in thread
From: Joseph Myers @ 2019-10-08 15:58 UTC (permalink / raw)
  To: Florian Weimer; +Cc: libc-alpha

On Tue, 8 Oct 2019, Florian Weimer wrote:

> +#ifdef CLONE_NEWPID
> +  if (unshare (CLONE_NEWPID) != 0)
> +    printf ("warning: unshare (CLONE_NEW_PID) failed: %m\n"
> +            "warning: This leads to reduced test coverage.\n");
> +  else
> +    pid_tests_supported = true;
> +#else
> +  printf ("warning: CLONE_NEW_PID not available.\n"

The messages are saying CLONE_NEW_PID when the actual code is using 
CLONE_NEWPID.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] wordexp: Split out command execution tests from posix/wordexp-test
  2019-10-08 15:58 ` Joseph Myers
@ 2019-10-08 17:00   ` Florian Weimer
  0 siblings, 0 replies; 4+ messages in thread
From: Florian Weimer @ 2019-10-08 17:00 UTC (permalink / raw)
  To: Joseph Myers; +Cc: libc-alpha

* Joseph Myers:

> On Tue, 8 Oct 2019, Florian Weimer wrote:
>
>> +#ifdef CLONE_NEWPID
>> +  if (unshare (CLONE_NEWPID) != 0)
>> +    printf ("warning: unshare (CLONE_NEW_PID) failed: %m\n"
>> +            "warning: This leads to reduced test coverage.\n");
>> +  else
>> +    pid_tests_supported = true;
>> +#else
>> +  printf ("warning: CLONE_NEW_PID not available.\n"
>
> The messages are saying CLONE_NEW_PID when the actual code is using 
> CLONE_NEWPID.

Sorry about that.  Fixed by the commit below.

Thanks,
Florian

8<------------------------------------------------------------------8<
Subject: posix/tst-wordexp-nocmd: Fix diagnostics output in test

2019-10-08  Florian Weimer  <fweimer@redhat.com>

	* posix/tst-wordexp-nocmd.c (do_test): Fix diagnostics output.

diff --git a/posix/tst-wordexp-nocmd.c b/posix/tst-wordexp-nocmd.c
index b2f64c819f..cb6033e15f 100644
--- a/posix/tst-wordexp-nocmd.c
+++ b/posix/tst-wordexp-nocmd.c
@@ -161,12 +161,12 @@ do_test (void)
 
 #ifdef CLONE_NEWPID
   if (unshare (CLONE_NEWPID) != 0)
-    printf ("warning: unshare (CLONE_NEW_PID) failed: %m\n"
+    printf ("warning: unshare (CLONE_NEWPID) failed: %m\n"
             "warning: This leads to reduced test coverage.\n");
   else
     pid_tests_supported = true;
 #else
-  printf ("warning: CLONE_NEW_PID not available.\n"
+  printf ("warning: CLONE_NEWPID not available.\n"
           "warning: This leads to reduced test coverage.\n");
 #endif
 

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2019-10-08 17:00 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-08 11:04 [PATCH] wordexp: Split out command execution tests from posix/wordexp-test Florian Weimer
2019-10-08 13:40 ` Adhemerval Zanella
2019-10-08 15:58 ` Joseph Myers
2019-10-08 17:00   ` Florian Weimer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).