]> sourceware.org Git - newlib-cygwin.git/commitdiff
Cygwin: execve: drop argument size limit
authorCorinna Vinschen <corinna@vinschen.de>
Tue, 29 Aug 2023 09:55:10 +0000 (11:55 +0200)
committerCorinna Vinschen <corinna@vinschen.de>
Tue, 29 Aug 2023 12:19:29 +0000 (14:19 +0200)
Before commit 44f73c5a6206 ("Cygwin: Fix segfalt when too many command
line args are specified.") we had no actual argument size limit, except
for the fact that the child process created another copy of the argv
array on the stack, which could result in a stack overflow and a
subsequent SEGV.  Commit 44f73c5a6206 changed that by allocating the
additional argv array via malloc, and it introduced a new SC_ARG_MAX
limit along the lines of the typical Linux limit.

However, this new limit is artificial. Cygwin allocates all argument
and environment data on the cygheap.  We only run out of ARG_MAX space
if we're out of memory resources.

Change argument size handling accordingly:
- Drop the args size check from  child_info_spawn::worker.
- Return -1 from sysconf (SC_ARG_MAX), i. e., the argument size limit
  is undefined.
- Change argv handling in class av, so that a failing cmalloc is not
  fatal.  This allows the parent process to return E2BIG if it's out
  of cygheap resources.
- In the child, add a check around the new malloc call, so that it
  doesn't result in a SEGV if the child process gets unexpectedly into
  an ENOMEM situation at this point. In this (unlikely) case, proceed
  with the original __argv array instead.  Add comment to explain why.

Fixes: 44f73c5a6206 ("Cygwin: Fix segfalt when too many command line args are specified.")
Tested-by: Takashi Yano <takashi.yano@nifty.ne.jp>
Signed-off-by: Corinna Vinschen <corinna@vinschen.de>
winsup/cygwin/dcrt0.cc
winsup/cygwin/kernel32.cc
winsup/cygwin/local_includes/winf.h
winsup/cygwin/local_includes/winsup.h
winsup/cygwin/spawn.cc
winsup/cygwin/sysconf.cc

index 1d88105463148b97834d1aee88fbc7589f488d22..130d652aac6eb6d76c0dec58906140b030e3b72e 100644 (file)
@@ -976,10 +976,22 @@ dll_crt0_1 (void *)
     {
       /* Create a copy of Cygwin's version of __argv so that, if the user makes
         a change to an element of argv[] it does not affect Cygwin's argv.
-        Changing the the contents of what argv[n] points to will still
-        affect Cygwin.  This is similar (but not exactly like) Linux. */
+        Changing the contents of what argv[n] points to will still affect
+        Cygwin.  This is similar (but not exactly like) Linux.
+
+        We used to allocate newargv on the stack, but all the rest of the
+        argument and environment handling does not depend on the stack,
+        as it does on Linux.  In fact, everything is stored by the parent
+        in the cygheap, so the only reason this may fail is if we're out
+        of resources in a big way.  If this malloc fails, we could either
+        fail the entire process execution, or we could proceed with the
+        original argv and potentially affect output of /proc/self/cmdline.
+        We opt for the latter here because it's the lesser evil. */
       char **newargv = (char **) malloc ((__argc + 1) * sizeof (char *));
-      memcpy (newargv, __argv, (__argc + 1) * sizeof (char *));
+      if (newargv)
+       memcpy (newargv, __argv, (__argc + 1) * sizeof (char *));
+      else
+       newargv = __argv;
       /* Handle any signals which may have arrived */
       sig_dispatch_pending (false);
       _my_tls.call_signal_handler ();
index 6248aefd5183379dd9821261aa6f0c4389ae8e52..36951f6a87be757a052ea426c4d24cdb2b88e0ee 100644 (file)
@@ -424,8 +424,11 @@ ucmd ()
       linebuf cmd;
       path_conv real_path (__argv[0]);
       av newargv (__argc, __argv);
-      cmd.fromargv (newargv, real_path.get_win32 (), true);
-      RtlInitUnicodeString (&wcmd, cmd);
+      if (newargv.argc)
+       {
+         cmd.fromargv (newargv, real_path.get_win32 (), true);
+         RtlInitUnicodeString (&wcmd, cmd);
+       }
     }
   return &wcmd;
 }
index 651f78ba2824607eac96c14db0a322a1e99b1749..b58693441095815830945692f3c071975d8e89f8 100644 (file)
@@ -23,11 +23,16 @@ class av
  public:
   int argc;
   bool win16_exe;
-  av (): argv (NULL) {}
-  av (int ac_in, const char * const *av_in) : calloced (0), argc (ac_in), win16_exe (false)
+  av () : argv (NULL), argc (0) {}
+  av (int ac_in, const char * const *av_in)
+  : calloced (0), win16_exe (false)
   {
-    argv = (char **) cmalloc_abort (HEAP_1_ARGV, (argc + 5) * sizeof (char *));
-    memcpy (argv, av_in, (argc + 1) * sizeof (char *));
+    argv = (char **) cmalloc (HEAP_1_ARGV, (ac_in + 5) * sizeof (char *));
+    if (argv)
+      {
+       argc = ac_in;
+       memcpy (argv, av_in, (argc + 1) * sizeof (char *));
+      }
   }
   void *operator new (size_t, void *p) __attribute__ ((nothrow)) {return p;}
   ~av ()
index 2d51722f3a8ec1d8ef86ae80d16e31f534aba73c..43dfbf46f49a7a99ec79da04c146a02ffc2da821 100644 (file)
@@ -68,10 +68,6 @@ uint32_t cygwin_inet_addr (const char *cp);
    application provided path strings we handle. */
 #define NT_MAX_PATH 32768
 
-/* CYG_ARG_MAX is the maximum total length of command line args.
-   The value 2097152 is the default ARG_MAX value in Linux. */
-#define CYG_ARG_MAX 2097152
-
 /* This definition allows to define wide char strings using macros as
    parameters.  See the definition of __CONCAT in newlib's sys/cdefs.h
    and accompanying comment. */
index c4888e823ba7aaf60b0c95320e95e8165babedea..5ac90808ca7007b0b2fbb5aa82492dc9f1700051 100644 (file)
@@ -340,9 +340,8 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
         We need to quote any argument that has whitespace or embedded "'s.  */
 
       int ac;
-      size_t arg_len = 0;
       for (ac = 0; argv[ac]; ac++)
-       arg_len += strlen (argv[ac]) + 1;
+       ;
 
       int err;
       const char *ext;
@@ -511,12 +510,6 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
          __leave;
        }
       set (chtype, real_path.iscygexec ());
-      if (iscygwin () && arg_len > (size_t) sysconf (_SC_ARG_MAX))
-       {
-         set_errno (E2BIG);
-         res = -1;
-         __leave;
-       }
       __stdin = in__stdin;
       __stdout = in__stdout;
       record_children ();
@@ -1119,11 +1112,16 @@ spawnvpe (int mode, const char *file, const char * const *argv,
 
 int
 av::setup (const char *prog_arg, path_conv& real_path, const char *ext,
-          int argc, const char *const *argv, bool p_type_exec)
+          int ac_in, const char *const *av_in, bool p_type_exec)
 {
   const char *p;
   bool exeext = ascii_strcasematch (ext, ".exe");
-  new (this) av (argc, argv);
+  new (this) av (ac_in, av_in);
+  if (!argc)
+    {
+      set_errno (E2BIG);
+      return -1;
+    }
   if ((exeext && real_path.iscygexec ()) || ascii_strcasematch (ext, ".bat")
       || (!*ext && ((p = ext - 4) > real_path.get_win32 ())
          && (ascii_strcasematch (p, ".bat") || ascii_strcasematch (p, ".cmd")
index 7cdfbdb9d40323b1816632c9bea4b59f5a7146c8..6529731a51c1fbb8f774acd8db60ce6fe141d5c1 100644 (file)
@@ -485,7 +485,7 @@ static struct
     };
 } sca[] =
 {
-  {cons, {c:CYG_ARG_MAX}},             /*   0, _SC_ARG_MAX */
+  {cons, {c:-1L}},                     /*   0, _SC_ARG_MAX */
   {cons, {c:CHILD_MAX}},               /*   1, _SC_CHILD_MAX */
   {cons, {c:CLOCKS_PER_SEC}},          /*   2, _SC_CLK_TCK */
   {cons, {c:NGROUPS_MAX}},             /*   3, _SC_NGROUPS_MAX */
This page took 0.039446 seconds and 5 git commands to generate.