parallel-checkout: fix stack buffer overflow in Windows poll() with many workers

Documentation/config/checkout.adoc

@@ -30,6 +30,11 @@ commands or functionality in the future.
 	all commands that perform checkout. E.g. checkout, clone, reset,
 	sparse-checkout, etc.
 +
+On Windows the number of workers is capped at 62, because the `poll()`
+emulation cannot wait on more worker pipes than that. A higher configured
+value, including the logical core count on a machine with many cores, is
+silently reduced to the cap.
++
 NOTE: Parallel checkout usually delivers better performance for repositories
 located on SSDs or over NFS. For repositories on spinning disks and/or machines
 with a small number of cores, the default sequential checkout often performs

compat/poll/poll.c

@@ -303,6 +303,40 @@ compute_revents (int fd, int sought, fd_set *rfds, fd_set *wfds, fd_set *efds)
 }
 #endif /* !MinGW */
 
+#ifdef WIN32_NATIVE
+/* POLL_MAX_DESCRIPTORS descriptors, plus hEvent and the QS_ALLINPUT message
+   queue, must fit in one MsgWaitForMultipleObjects call, and the collected
+   handles plus the NULL sentinel must fit in handle_array.  */
+#if POLL_MAX_DESCRIPTORS + 2 > MAXIMUM_WAIT_OBJECTS
+#error POLL_MAX_DESCRIPTORS exceeds MAXIMUM_WAIT_OBJECTS
+#endif
+#if POLL_MAX_DESCRIPTORS + 2 > FD_SETSIZE + 2
+#error POLL_MAX_DESCRIPTORS does not fit in handle_array
+#endif
+
+/* Undo the WSAEventSelect() calls made for the first NFD descriptors.  */
+static void
+reset_socket_events (struct pollfd *pfd, nfds_t nfd)
+{
+  nfds_t i;
+
+  for (i = 0; i < nfd; i++)
+    {
+      HANDLE h;
+
+      if (pfd[i].fd < 0)
+	continue;
+
+      h = (HANDLE) _get_osfhandle (pfd[i].fd);
+      if (h == NULL || h == INVALID_HANDLE_VALUE)
+	continue;
+
+      if (IsSocketHandle (h))
+	WSAEventSelect ((SOCKET) h, NULL, 0);
+    }
+}
+#endif
+
 int
 poll (struct pollfd *pfd, nfds_t nfd, int timeout)
 {
@@ -504,7 +538,16 @@ poll (struct pollfd *pfd, nfds_t nfd, int timeout)
 	     bits for the "wrong" direction. */
 	  pfd[i].revents = win32_compute_revents (h, &sought);
 	  if (sought)
-	    handle_array[nhandles++] = h;
+	    {
+	      /* hEvent occupies handle_array[0].  See POLL_MAX_DESCRIPTORS.  */
+	      if (nhandles > POLL_MAX_DESCRIPTORS)
+		{
+		  reset_socket_events (pfd, i);
+		  errno = EINVAL;
+		  return -1;
+		}
+	      handle_array[nhandles++] = h;
+	    }
 	  if (pfd[i].revents)
 	    timeout = 0;
 	}

compat/poll/poll.h

@@ -59,6 +59,20 @@ typedef unsigned long nfds_t;
 
 extern int poll (struct pollfd *pfd, nfds_t nfd, int timeout);
 
+/*
+ * This poll() is emulated with MsgWaitForMultipleObjects(), which waits on at
+ * most MAXIMUM_WAIT_OBJECTS (64) objects. Two of those are never available for
+ * polled descriptors: poll() waits on its own event object, and QS_ALLINPUT
+ * adds the thread message queue. Sockets do not count, because they are all
+ * multiplexed onto that one event object; every other descriptor takes a wait
+ * slot of its own.
+ *
+ * Callers that poll one or more descriptors per child must keep the number of
+ * simultaneously live descriptors within this limit. Exceeding it fails with
+ * EINVAL.
+ */
+#define POLL_MAX_DESCRIPTORS 62
+
 /* Define INFTIM only if doing so conforms to POSIX.  */
 #if !defined (_POSIX_C_SOURCE) && !defined (_XOPEN_SOURCE)
 #define INFTIM (-1)

compat/posix.h

@@ -133,6 +133,16 @@
 /* Pull the compat stuff */
 #include <poll.h>
 #endif
+
+/*
+ * compat/poll defines POLL_MAX_DESCRIPTORS to the largest number of
+ * descriptors its poll() emulation can wait on. A native poll() has no such
+ * limit, so callers that fan out one descriptor per child can clamp against
+ * this unconditionally.
+ */
+#ifndef POLL_MAX_DESCRIPTORS
+#define POLL_MAX_DESCRIPTORS INT_MAX
+#endif
 #ifdef HAVE_BSD_SYSCTL
 #include <sys/sysctl.h>
 #endif

parallel-checkout.c

@@ -671,6 +671,13 @@ int run_parallel_checkout(struct checkout *state, int num_workers, int threshold
 	if (parallel_checkout.nr < num_workers)
 		num_workers = parallel_checkout.nr;
 
+	/*
+	 * gather_results_from_workers() polls one pipe per worker, so the
+	 * worker count must stay within what poll() can wait on.
+	 */
+	if (num_workers > POLL_MAX_DESCRIPTORS)
+		num_workers = POLL_MAX_DESCRIPTORS;
+
 	if (num_workers <= 1 || parallel_checkout.nr < threshold) {
 		write_items_sequentially(state);
 	} else {

run-command.c

@@ -1894,6 +1894,7 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts)
 	int i, code;
 	int timeout = 100;
 	int spawn_cap = 4;
+	size_t max_live;
 	struct parallel_processes_for_signal pp_sig;
 	struct parallel_processes pp = {
 		.buffered_output = STRBUF_INIT,
@@ -1903,6 +1904,18 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts)
 	const char *tr2_label = opts->tr2_label;
 	const int do_trace2 = tr2_category && tr2_label;
 
+	/*
+	 * Unless the caller handles its own output, pp_buffer_io() polls one
+	 * pipe for each child that is sending output and a second one for each
+	 * child that is being fed on stdin. Limit how many children run at once
+	 * so that the worst case stays within what poll() can wait on. Only
+	 * concurrency is limited; the configured maximum is still honoured for
+	 * the number of tasks that are run in total.
+	 */
+	max_live = opts->processes;
+	if (!opts->ungroup && max_live > POLL_MAX_DESCRIPTORS / 2)
+		max_live = POLL_MAX_DESCRIPTORS / 2;
+
 	if (do_trace2)
 		trace2_region_enter_printf(tr2_category, tr2_label, NULL,
 					   "max:%"PRIuMAX,
@@ -1924,7 +1937,7 @@ void run_processes_parallel(const struct run_process_parallel_opts *opts)
 	while (1) {
 		for (i = 0;
 		    i < spawn_cap && !pp.shutdown &&
-		    pp.nr_processes < opts->processes;
+		    pp.nr_processes < max_live;
 		    i++) {
 			code = pp_start_one(&pp, opts);
 			if (!code)

t/t2080-parallel-checkout-basics.sh

@@ -319,5 +319,41 @@ test_expect_success MINGW 'parallel checkout with fscache does not fail on new d
 		test_cmp expect2 sub/deep/dir/file2.txt
 	)
 '
+# Windows has no native poll(). compat/poll emulates it with
+# MsgWaitForMultipleObjects(), which cannot wait on more than
+# MAXIMUM_WAIT_OBJECTS objects, so run_parallel_checkout() caps the worker
+# count at MAXIMUM_WAIT_OBJECTS - 2. Without that cap, compat/poll collected
+# one wait handle per polled worker pipe in a fixed-size stack array and
+# smashed the stack.
+#
+# MAXIMUM_WAIT_OBJECTS is 64, hence the expected 62 below. The test is
+# MINGW-only because the cap only exists there; on other platforms the
+# requested 200 workers are used as-is.
+test_expect_success MINGW 'checkout caps workers at the poll limit' '
+	test_when_finished "rm -rf many-workers" &&
+	git init many-workers &&
+	(
+		cd many-workers &&
+		mkdir dir &&
+		for i in $(test_seq 1 200)
+		do
+			echo "content $i" >dir/file$i || return 1
+		done &&
+		git add -A &&
+		git commit -q -m base &&
+
+		git checkout -q -b other &&
+		for i in $(test_seq 1 200)
+		do
+			echo "changed $i" >dir/file$i || return 1
+		done &&
+		git commit -q -a -m changed &&
+		git checkout -q -
+	) &&
+
+	set_checkout_config 200 1 &&
+	test_checkout_workers 62 git -C many-workers checkout other &&
+	verify_checkout many-workers
+'
 
 test_done