Skip to content

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

Open
tyrielv wants to merge 3 commits into
microsoft:vfs-2.55.0from
tyrielv:vfs-fix-poll-worker-overflow
Open

parallel-checkout: fix stack buffer overflow in Windows poll() with many workers#981
tyrielv wants to merge 3 commits into
microsoft:vfs-2.55.0from
tyrielv:vfs-fix-poll-worker-overflow

Conversation

@tyrielv

@tyrielv tyrielv commented Sep 1, 2026

Copy link
Copy Markdown

This fork contains changes specific to monorepo scenarios. If you are an
external contributor, then please detail your reason for submitting to
this fork:

  • This is an early version of work already under review upstream.
  • This change only applies to interactions with Azure DevOps and the
    GVFS Protocol.
  • This change only applies to the virtualization hook and VFS for Git.

Upstream: git-for-windows#6395
(git-for-windows#6395) carries the same three
commits against main. This PR takes them early so the fix reaches VFS for
Git users before the next rebase onto git-for-windows.


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 where the limit lives

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.

That value is defined once, as POLL_MAX_DESCRIPTORS in compat/poll/poll.h
alongside the poll() declaration it constrains, and both callers clamp against
it. compat/posix.h defines it to INT_MAX where a native poll() is used, so
the callers need no #ifdef.

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. compat/poll: do not collect more handles than the wait supports — defines
    POLL_MAX_DESCRIPTORS (62) next to the poll() declaration and refuses to
    collect beyond it, returning EINVAL instead of appending past the end of the
    array. Two preprocessor assertions tie the constant to MAXIMUM_WAIT_OBJECTS
    and to the size of handle_array, so they cannot drift apart. poll() is now
    memory-safe for every input.

    The error path also undoes the WSAEventSelect() registrations made earlier in
    the same call. The loop that normally does that runs after the wait, so
    returning early would otherwise leave those sockets bound to poll()'s static
    event object and let later socket activity disturb an unrelated poll().

    The limit 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.

    On platforms with a native poll() there is no such limit, so
    POLL_MAX_DESCRIPTORS is INT_MAX and callers can clamp against it
    unconditionally.

  2. parallel-checkout: limit worker count to what poll() can wait on — clamps
    num_workers to POLL_MAX_DESCRIPTORS 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. Also
    documents the cap, since checkout.workers was described as using one worker
    per logical core with no upper bound.

  3. run-command: limit concurrent children to what poll() can wait on — the
    same limit applied to the other poll() fan-out. pp_buffer_io() polls one
    pipe per child sending output and a second per child being fed on stdin, and
    fetch.parallel / submodule.fetchJobs / hook.jobs all accept a high value
    (or 0, meaning online_cpus()). Without this, change 1 would convert the old
    stack smash on that path into a hard die_errno("poll"). Only concurrency is
    limited; the configured maximum still sizes the arrays and is still reported by
    the trace, so the total number of tasks run is unchanged.

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 the effective worker count rather
than a crash. test_checkout_workers counts the workers actually spawned from a
TRACE2 log, so the test verifies the clamp took effect (62) instead of merely
checking that the checkout did not crash. It is MINGW-gated, because that is the
only platform where the cap applies.

Testing

  • New test in t/t2080-parallel-checkout-basics.sh. t2080, t2081, t2082,
    t0061, t5526 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
tyrielv force-pushed the vfs-fix-poll-worker-overflow branch 2 times, most recently from 04e80b5 to 7543596 Compare September 1, 2026 19:14
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. One further handle is poll()'s own event
object. So at most MAXIMUM_WAIT_OBJECTS - 2 descriptors can be waited on,
which is the tighter of the two bounds and is well inside the array.

Define that limit as POLL_MAX_DESCRIPTORS next to the poll() declaration,
and refuse to collect beyond it, returning EINVAL. Two preprocessor
checks tie the constant to MAXIMUM_WAIT_OBJECTS and to the size of
handle_array, so the two cannot drift apart. poll() is now memory-safe
for every input, and a case that previously smashed the stack fails
cleanly.

Undo the WSAEventSelect() registrations before returning. The loop that
normally does this runs after the wait, and the new error path skips it,
which would otherwise leave those sockets associated with poll()'s static
event object and let later socket activity disturb an unrelated poll().

Note that the limit 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. Sockets are
all multiplexed onto the one event object. 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.

For platforms with a native poll(), which has no such limit, define
POLL_MAX_DESCRIPTORS to INT_MAX so that callers can clamp against it
unconditionally.

Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
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(), which cannot wait on more than
POLL_MAX_DESCRIPTORS descriptors at once. compat/poll collects one handle
per polled descriptor in a fixed stack array, so a higher worker count
writes past the end of that array and corrupts the stack.

run_parallel_checkout() clamped num_workers only by the number of files.
Clamp it to POLL_MAX_DESCRIPTORS as well. That function 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 poll() loop cannot usefully
drive more readers than this anyway. On platforms with a native poll()
the limit is INT_MAX, so the clamp is a no-op.

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.

Document the cap, because checkout.workers is otherwise described as
using one worker per logical core with no upper bound.

Add a test that asserts the clamp. test_checkout_workers() counts the
workers that were actually spawned, so the test can check the effective
count rather than only that the checkout succeeded. The test is limited
to Windows, because that is the only platform where the cap applies.

Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
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. On Windows poll() is
emulated with MsgWaitForMultipleObjects(), which cannot wait on more than
POLL_MAX_DESCRIPTORS descriptors at once.

A job count above that limit is reachable in practice. fetch.parallel,
submodule.fetchJobs and hook.jobs all accept an explicit value, and a value
of 0 means "use online_cpus()", which on a machine with many cores is well
above the limit. Before the previous commit such a run corrupted the stack.
Now poll() returns EINVAL, and pp_buffer_io() turns that into
die_errno("poll"), so the operation fails outright.

Limit how many children run at the same time, so that a large job count
degrades into less concurrency instead of an error. Only concurrency is
limited. The configured maximum is still used for the size of the child and
pollfd arrays, and is still reported by the trace, so the number of tasks
that are run in total does not change. Unused pollfd slots hold -1 and are
skipped by poll(), so the larger array costs nothing.

Divide the limit by two, because a child can hold two descriptors: one for
its output and one for its input. Callers that group output are the only ones
affected; with opts.ungroup set the caller does its own I/O and poll() is not
involved.

On platforms with a native poll() there is no such limit,
POLL_MAX_DESCRIPTORS is INT_MAX, and this is a no-op.

Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
@tyrielv
tyrielv force-pushed the vfs-fix-poll-worker-overflow branch from 7543596 to 6ca8024 Compare September 1, 2026 20:35
@tyrielv
tyrielv marked this pull request as ready for review September 1, 2026 23:14
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