public inbox for cygwin-cvs@sourceware.org
help / color / mirror / Atom feed
* [newlib-cygwin/cygwin-3_4-branch] Cygwin: Fix segfalt when too many command line args are specified.
@ 2023-08-28 16:00 Takashi Yano
  0 siblings, 0 replies; only message in thread
From: Takashi Yano @ 2023-08-28 16:00 UTC (permalink / raw)
  To: cygwin-cvs

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=33cddf77974f9849cf8edafe9b3da46591b73d6e

commit 33cddf77974f9849cf8edafe9b3da46591b73d6e
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Mon Aug 28 22:14:41 2023 +0900

    Cygwin: Fix segfalt when too many command line args are specified.
    
    Previously, the number of command line args was not checked for
    cygwin process. Due to this, segmentation fault was caused if too
    many command line args are specified.
    https://cygwin.com/pipermail/cygwin/2023-August/254333.html
    
    Since char *argv[argc + 1] is placed on the stack in dll_crt0_1(),
    STATUS_STACK_OVERFLOW occurs if the stack does not have enough
    space.
    
    With this patch, char *argv[] is placed in heap instead of stack
    and ARG_MAX is increased from 32000 to 2097152 which is default
    value of Linux. The argument length is also compared with ARG_MAX
    and spawnve() returns E2BIG if it is too long.
    
    Reported-by: Ed Morton
    Reviewed-by: Corinna Vinschen <corinna@vinschen.de>
    Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>

Diff:
---
 winsup/cygwin/dcrt0.cc                | 7 ++-----
 winsup/cygwin/local_includes/winsup.h | 4 ++++
 winsup/cygwin/release/3.4.9           | 3 +++
 winsup/cygwin/spawn.cc                | 9 ++++++++-
 winsup/cygwin/sysconf.cc              | 2 +-
 5 files changed, 18 insertions(+), 7 deletions(-)

diff --git a/winsup/cygwin/dcrt0.cc b/winsup/cygwin/dcrt0.cc
index 49b7a44ae..1d8810546 100644
--- a/winsup/cygwin/dcrt0.cc
+++ b/winsup/cygwin/dcrt0.cc
@@ -978,11 +978,8 @@ dll_crt0_1 (void *)
 	 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. */
-      char *newargv[__argc + 1];
-      char **nav = newargv;
-      char **oav = __argv;
-      while ((*nav++ = *oav++) != NULL)
-	continue;
+      char **newargv = (char **) malloc ((__argc + 1) * sizeof (char *));
+      memcpy (newargv, __argv, (__argc + 1) * sizeof (char *));
       /* Handle any signals which may have arrived */
       sig_dispatch_pending (false);
       _my_tls.call_signal_handler ();
diff --git a/winsup/cygwin/local_includes/winsup.h b/winsup/cygwin/local_includes/winsup.h
index 43dfbf46f..2d51722f3 100644
--- a/winsup/cygwin/local_includes/winsup.h
+++ b/winsup/cygwin/local_includes/winsup.h
@@ -68,6 +68,10 @@ 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. */
diff --git a/winsup/cygwin/release/3.4.9 b/winsup/cygwin/release/3.4.9
index 2f2da9e13..53c4e5fc8 100644
--- a/winsup/cygwin/release/3.4.9
+++ b/winsup/cygwin/release/3.4.9
@@ -8,3 +8,6 @@ Bug Fixes
 - For the time being, disable creating special files using mknod/mkfifo
   on NFS.
   Addresses: https://cygwin.com/pipermail/cygwin/2023-August/254266.html
+
+- Fix segfault when too many command line args are specified.
+  Addresses: https://cygwin.com/pipermail/cygwin/2023-August/254333.html
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 32ba5b377..c4888e823 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -340,8 +340,9 @@ 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++)
-	/* nothing */;
+	arg_len += strlen (argv[ac]) + 1;
 
       int err;
       const char *ext;
@@ -510,6 +511,12 @@ 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 ();
diff --git a/winsup/cygwin/sysconf.cc b/winsup/cygwin/sysconf.cc
index 2db92e4de..7cdfbdb9d 100644
--- a/winsup/cygwin/sysconf.cc
+++ b/winsup/cygwin/sysconf.cc
@@ -485,7 +485,7 @@ static struct
     };
 } sca[] =
 {
-  {cons, {c:ARG_MAX}},			/*   0, _SC_ARG_MAX */
+  {cons, {c:CYG_ARG_MAX}},		/*   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 */

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2023-08-28 16:00 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-28 16:00 [newlib-cygwin/cygwin-3_4-branch] Cygwin: Fix segfalt when too many command line args are specified Takashi Yano

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).