Skip to content

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

Draft
tyrielv wants to merge 2 commits into
git-for-windows:mainfrom
tyrielv:gfw-fix-poll-worker-overflow
Draft

parallel-checkout: fix stack buffer overflow in Windows poll() with many workers#6395
tyrielv wants to merge 2 commits into
git-for-windows:mainfrom
tyrielv:gfw-fix-poll-worker-overflow

Conversation

@tyrielv

@tyrielv tyrielv commented Sep 1, 2026

Copy link
Copy Markdown

Symptom

On Windows, git checkout and git reset --hard can abort with

*** stack smashing detected ***: terminated

and exit code 0xC0000409 (STATUS_STACK_BUFFER_OVERRUN). This is memory
corruption, not a normal error. The process dies before Trace2 writes its log,
so nothing shows up in a trace. A .git/index.lock is left behind.

It happens when checkout.workers is large, or when it is 0 (meaning "use
online_cpus()") on a machine with many logical processors.

Mechanism

gather_results_from_workers() in parallel-checkout.c polls one pipe per
checkout worker:

CALLOC_ARRAY(pfds, num_workers);
...
poll(pfds, num_workers, -1);

Windows has no native poll(), so compat/poll/poll.c emulates it with
MsgWaitForMultipleObjects(). It collects one handle per polled descriptor in a
fixed stack array:

HANDLE h, handle_array[FD_SETSIZE + 2];   /* 64 + 2 = 66 entries */
...
handle_array[nhandles++] = h;             /* no bounds check */
...
handle_array[nhandles] = NULL;            /* sentinel, no bounds check */

FD_SETSIZE is the Winsock default 64, and nothing in the build overrides it.
run_parallel_checkout() clamps num_workers only against the number of files,
never against the array size or the Windows wait limit. A high worker count
therefore writes past the end of the array and smashes the stack.

Sockets are not involved: they are multiplexed onto a single event through
WSAEventSelect, so only non-socket descriptors consume a slot.

Why 62 and not 64

Two of the wait slots are never available for descriptors:

  • compat/poll uses index 0 for its own event object.
  • QS_ALLINPUT adds the thread message queue as an implicit wait object. The
    code confirms this, because it reports the message queue as
    WAIT_OBJECT_0 + nhandles.

So nhandles + 1 <= MAXIMUM_WAIT_OBJECTS, which gives at most
MAXIMUM_WAIT_OBJECTS - 2 = 62 descriptors.

Why the array cannot simply be enlarged

MAXIMUM_WAIT_OBJECTS is a kernel limit, not a header convenience. Passing more
handles fails with ERROR_INVALID_PARAMETER. Growing the array would only turn
memory corruption into a functional failure. Support for more descriptors needs
a wait tree (helper threads each waiting on at most 62 handles) or completion
ports, which is out of scope here.

Why it surfaced in 2.54

parallel-checkout.c and compat/poll/poll.c are unchanged between 2.53 and
2.54. Only online_cpus() changed:

Version API Result
2.53 GetSystemInfo() processors in the current processor group only; a group holds at most 64
2.54+ GetLogicalProcessorInformationEx() true system-wide logical processor count

The old API could never report more than 64, so the array always fit. That
ceiling was accidental, not deliberate. The online_cpus() change is correct and
must stay; it only exposed a latent bug.

The changes

  1. parallel-checkout: limit worker count on Windows — clamp num_workers
    to MAXIMUM_WAIT_OBJECTS - 2 in run_parallel_checkout(), the single choke
    point before the workers start. The clamp is silent: fewer workers is correct,
    and a warning would fire on every checkout on a large machine. There is no
    measurable cost, because a single-threaded poll() loop cannot usefully drive
    more concurrent pipe readers than that.

  2. compat/poll: do not collect more handles than the wait supports — bound
    the number of collected handles by MAXIMUM_WAIT_OBJECTS - 1 and return
    EINVAL instead of appending past the end of the array. poll() is then
    memory-safe for every input, and a case that previously smashed the stack now
    fails cleanly.

    The bound is on the 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 —
    run_processes_parallel() sizes its pollfd array to the configured job count
    and leaves the unused slots at fd = -1. An earlier revision of this PR
    rejected a large nfd instead, which broke t7406 (submodule.fetchJobs 67,
    with only a handful of live pipes) with fatal: poll: Invalid argument.

The two changes address different bounds. The poll() guard makes the emulation
safe for any caller. The clamp keeps parallel checkout inside the wait limit so
it keeps using workers instead of failing.

Reproduction

No clone, no special hardware, about 10 seconds. A many-core machine is not
required: a positive checkout.workers is used verbatim, and online_cpus() is
consulted only when the value is 0 or less.

# Use a NEW directory every attempt (see the note on timing below).
$repo = "C:\tmp\poll-repro-$(Get-Random)"
New-Item -ItemType Directory -Force -Path $repo | Out-Null
Set-Location $repo

git init -q -b main .
git config user.email repro@example.com
git config user.name  repro
git config checkout.workers 200
git config checkout.thresholdForParallelism 1

New-Item -ItemType Directory -Force -Path dir | Out-Null
1..400 | ForEach-Object { Set-Content -Path "dir\f$_.txt" -Value "base $_" -NoNewline }
git add -A; git commit -qm base

git checkout -qb other
1..400 | ForEach-Object { Set-Content -Path "dir\f$_.txt" -Value "changed $_ padding padding padding" -NoNewline }
git commit -qam changed

git checkout -q main
Write-Host "exit=$LASTEXITCODE"

Before the fix, on a 12-core machine, this crashed 3 out of 3 runs with
exit=-1073740791 (0xC0000409) and left .git/index.lock behind. After the
fix it exits 0 on 3 out of 3 runs, with the files correctly updated. A
checkout.workers 16 checkout still works, as before.

The crash is not deterministic

poll() only appends a descriptor when the worker's pipe has no data ready yet,
so nhandles reflects the workers pending at that instant, not the workers
spawned. On warm cache, pipes answer immediately and few workers stay pending.
Measured before the fix:

workers result
16, 64, 65, 70, 72, 74, 76, 78 pass
80 crashed once, then passed 3 times
200, repeated checkouts in the same repo passed 4 times
200, fresh repository each run crashed 3 of 3

The first out-of-bounds write happens at 65 descriptors by arithmetic, but the
corruption does not reliably reach the stack cookie until well past that. The
corruption is real from 65 onward whether or not it crashes. That is why the fix
targets the contract (62), not the observed crash point.

For the same reason, the added test asserts that a high worker count succeeds.
It does not verify that the clamp happens (the clamp is not reported anywhere),
and it does not assert that any particular worker count fails without the fix,
because such a test would be flaky.

Testing

  • New test in t/t2080-parallel-checkout-basics.sh; t2080, t2081, t2082
    and t7406 all pass on Windows.
  • Manual verification with the reproduction above, plus a low-worker-count
    regression check.

Workaround for affected users

git config checkout.workers 16

Any value at or below 62 avoids the overflow. No downgrade is needed.

tyrielv and others added 2 commits August 31, 2026 19:45
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 <tyrielv@gmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 <tyrielv@gmail.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@tyrielv
tyrielv force-pushed the gfw-fix-poll-worker-overflow branch from 78f5415 to 0b5d87d Compare September 1, 2026 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant