From 3db7ec9bf65bd9bfd63f6d6b895fb9777846e5ab Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Mon, 31 Aug 2026 19:44:49 -0700 Subject: [PATCH 1/2] parallel-checkout: limit worker count on Windows On Windows, `git checkout` and `git reset --hard` can abort with *** stack smashing detected ***: terminated and exit code 0xC0000409 (STATUS_STACK_BUFFER_OVERRUN) when checkout.workers is large, or when it is set to 0 on a machine with many logical processors. gather_results_from_workers() polls one pipe per checkout worker. Windows has no native poll(), so compat/poll emulates it with MsgWaitForMultipleObjects(). That function waits on at most MAXIMUM_WAIT_OBJECTS objects, and compat/poll collects one handle per polled descriptor in a fixed stack array, without a bounds check. A high worker count therefore writes past the end of that array and corrupts the stack. Two of the wait slots are not available for descriptors: compat/poll uses the first for its own event object, and QS_ALLINPUT adds the thread message queue as an implicit object. The code confirms this, because it reports the message queue as WAIT_OBJECT_0 + nhandles. So the usable limit is MAXIMUM_WAIT_OBJECTS - 2 descriptors. Clamp the worker count to that limit in run_parallel_checkout(), which is the single choke point before the workers start and the poll() loop runs. Clamp silently: fewer workers is correct behaviour, and a warning would fire on every checkout on a large machine. A single-threaded poll() loop cannot usefully drive more readers than this anyway. Enlarging the array does not help. MAXIMUM_WAIT_OBJECTS is a kernel limit, so passing more handles fails with ERROR_INVALID_PARAMETER. That would replace memory corruption with a functional failure. Support for more descriptors needs a wait tree or completion ports, which is out of scope here. The problem became reachable in 2.54. Before that, online_cpus() used GetSystemInfo(), which reports only the processors in the current processor group, and a group holds at most 64. That accidental ceiling kept the array in bounds. The move to GetLogicalProcessorInformationEx() is correct and reports the true system-wide count, which exposed the latent bug. Add a test that runs a checkout with a very high worker count and asserts that it succeeds. The test does not verify that the clamp happens, and it cannot easily do so: the clamp is not reported anywhere. It also cannot assert that any particular worker count fails without the clamp, because poll() only takes a handle for a worker whose pipe has no data yet, so the number of handles depends on timing and I/O state. Signed-off-by: Tyrie Vella Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- parallel-checkout.c | 17 +++++++++++++ t/t2080-parallel-checkout-basics.sh | 37 +++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/parallel-checkout.c b/parallel-checkout.c index 1eb277a0fc0a55..5f27143d6cf532 100644 --- a/parallel-checkout.c +++ b/parallel-checkout.c @@ -41,6 +41,18 @@ enum pc_status parallel_checkout_status(void) static const int DEFAULT_THRESHOLD_FOR_PARALLELISM = 100; static const int DEFAULT_NUM_WORKERS = 1; +#ifdef GIT_WINDOWS_NATIVE +/* + * Windows has no native poll(). compat/poll emulates it with + * MsgWaitForMultipleObjects(), which waits on at most MAXIMUM_WAIT_OBJECTS + * objects. One of those is the message queue implied by QS_ALLINPUT, and + * compat/poll reserves another for its own event object, leaving room for + * MAXIMUM_WAIT_OBJECTS - 2 descriptors. gather_results_from_workers() polls + * one pipe per worker, so the worker count must stay within that limit. + */ +#define MAX_PARALLEL_CHECKOUT_WORKERS (MAXIMUM_WAIT_OBJECTS - 2) +#endif + void get_parallel_checkout_configs(int *num_workers, int *threshold) { char *env_workers = getenv("GIT_TEST_CHECKOUT_WORKERS"); @@ -671,6 +683,11 @@ int run_parallel_checkout(struct checkout *state, int num_workers, int threshold if (parallel_checkout.nr < num_workers) num_workers = parallel_checkout.nr; +#ifdef GIT_WINDOWS_NATIVE + if (num_workers > MAX_PARALLEL_CHECKOUT_WORKERS) + num_workers = MAX_PARALLEL_CHECKOUT_WORKERS; +#endif + if (num_workers <= 1 || parallel_checkout.nr < threshold) { write_items_sequentially(state); } else { diff --git a/t/t2080-parallel-checkout-basics.sh b/t/t2080-parallel-checkout-basics.sh index 7ad96cd5cd24a3..cf0d822fd708de 100755 --- a/t/t2080-parallel-checkout-basics.sh +++ b/t/t2080-parallel-checkout-basics.sh @@ -320,4 +320,41 @@ test_expect_success MINGW 'parallel checkout with fscache does not fail on new d ) ' +# A worker count far above any platform limit must still produce a correct +# checkout. On Windows this used to smash the stack, because compat/poll +# collected one wait handle per polled worker pipe in a fixed-size array. +# +# This test does not verify that clamping happens, and it cannot easily do so: +# the clamp is not reported anywhere, and the number of workers that are +# actually pending inside poll() depends on timing and I/O state, so no +# specific worker count reliably demonstrates the old failure either. All the +# test asserts is that a high worker count succeeds. +test_expect_success 'checkout with more workers than the platform can poll' ' + test_when_finished "rm -rf many-workers-repo" && + git init many-workers-repo && + ( + cd many-workers-repo && + mkdir -p 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 config checkout.workers 200 && + git config checkout.thresholdForParallelism 1 && + + git checkout -q - && + git checkout -q other && + test "$(cat dir/file1)" = "changed 1" + ) +' + test_done From 04e80b50b1b81e971f3a1b423e87c80344f73d4d Mon Sep 17 00:00:00 2001 From: Tyrie Vella Date: Mon, 31 Aug 2026 19:44:50 -0700 Subject: [PATCH 2/2] compat/poll: do not collect more handles than the wait supports The Windows implementation of poll() collects one wait handle per polled descriptor in HANDLE h, handle_array[FD_SETSIZE + 2]; and appends to it without a bounds check. It then writes a NULL sentinel at handle_array[nhandles]. A caller with enough live descriptors therefore writes past the end of the array and corrupts the stack. The corruption is silent, and when it reaches the stack cookie the process aborts with STATUS_STACK_BUFFER_OVERRUN. The array is not the only limit. The collected handles are passed to MsgWaitForMultipleObjects (nhandles, handle_array, FALSE, wait_timeout, QS_ALLINPUT); which waits on at most MAXIMUM_WAIT_OBJECTS objects, and QS_ALLINPUT adds the thread message queue as one more object beyond the handles. The code shows this, because it reports the message queue as WAIT_OBJECT_0 + nhandles. So at most MAXIMUM_WAIT_OBJECTS - 1 handles can be collected, which is the tighter of the two bounds and is well inside the array. Refuse to collect beyond that, and return EINVAL. This makes poll() memory-safe for every input, and turns a case that previously smashed the stack into a clean error. Note that the bound is on the number of handles actually collected, not on nfd. Those are different: a descriptor only takes a handle when it is non-negative, is not a socket, and has no events pending yet. Callers routinely pass sparse arrays, for example run_processes_parallel(), which sizes its pollfd array to the configured job count and leaves the unused slots at fd = -1. Rejecting a large nfd would break such callers even though they never come close to the wait limit. Signed-off-by: Tyrie Vella Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- compat/poll/poll.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/compat/poll/poll.c b/compat/poll/poll.c index ea362b4a8e2340..cc61b2704c7bd4 100644 --- a/compat/poll/poll.c +++ b/compat/poll/poll.c @@ -504,7 +504,21 @@ 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; + { + /* + * MsgWaitForMultipleObjects() below waits on the handles + * collected here plus the message queue implied by + * QS_ALLINPUT, so at most MAXIMUM_WAIT_OBJECTS - 1 handles + * fit. Refuse to collect more instead of writing past the + * end of handle_array. + */ + if (nhandles >= MAXIMUM_WAIT_OBJECTS - 1) + { + errno = EINVAL; + return -1; + } + handle_array[nhandles++] = h; + } if (pfd[i].revents) timeout = 0; }