Uh oh!
There was an error while loading. Please reload this page.
Fix OOM state-desync in generated array-member setters - #2414
Closed
rparolin wants to merge 1 commit into
Closed
Conversation
|
rparolinforce-pushed
the
fix/array-setter-oom-strong-guarantee
branch
from
July 23, 2026 22:10
2ff5f72 to
1136fdaCompareThe generated setters for list-valued struct members (CUlaunchConfig.attrs, CUDA_MEM_ALLOC_NODE_PARAMS.accessDescs, ...) freed the old buffer, then allocated the new one; on allocation failure they raised *before* updating the cached length and the C-struct pointer. That left the wrapper with a freed buffer but a stale length and a dangling _pvt_ptr[0].<m>: - a later same-length assignment took the no-resize path and memcpy'd through the freed/NULL buffer -> near-NULL segfault, and - getPtr()-then-CUDA read the dangling pointer -> use-after-free. The setters now allocate and fill a new buffer first, and free/swap the old one only once the resize is known to succeed, so a failed (over)allocation leaves the object completely unchanged (strong exception guarantee). The fix is made in the cybind generator template (legacy_cython_gen). These files are the full regenerated output of python -m cybind --generate-module "cuda.*" \ --output-dir <checkout>/cuda_bindings --ctk-target-version 13.3 so besides the setter change (18 driver + 11 runtime setters) the regen also picks up accumulated docstring-only cleanups that have landed in cybind since the last sync (no API/behavior change). Adds a subprocess regression test that forces the failed resize via an overflowing __len__. Addresses Glasswing finding V1.1 (NVBUG 6268871). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
rparolinforce-pushed
the
fix/array-setter-oom-strong-guarantee
branch
from
July 23, 2026 22:17
1136fda to
497d561Comparemdboom
approved these changes
Jul 23, 2026
| CUDA_EMULATION_STRATEGY_EAGER | ||
| ctypedef cudaEmulationStrategy_t cudaEmulationStrategy | ||
| cdef extern from 'library_types.h': |
Contributor
There was a problem hiding this comment.
Looks like maybe there is a non-deterministic ordering bug in the generator here. Not to fix now, but I will file a bug over there.
| self._pvt_ptr[0].paramsArray = _paramsArray_new | ||
| else: | ||
| for idx in range(len(val)): | ||
| string.memcpy(&self._paramsArray[idx], (<cudaExternalSemaphoreWaitParams>val[idx])._pvt_ptr, sizeof(cyruntime.cudaExternalSemaphoreWaitParams)) |
Contributor
There was a problem hiding this comment.
We really should refactor this out into a function to remove all this duplication, but the legacy generator makes that hard and probably not worth the effort for something that's being deprecated.
mdboom
approved these changes
Jul 23, 2026
CollaboratorAuthor
Closing as superseded by #2423. The setter fix in this PR already landed on This branch is a CTK 13.3 regen, so rebasing it onto 13.4 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The generated setters for list-valued struct members (
CUlaunchConfig.attrs,CUDA_MEM_ALLOC_NODE_PARAMS.accessDescs, and ~27 siblings) freed the old heap buffer before allocating the new one. On an allocation failure they raisedMemoryErrorbefore updating the cached length and the C-struct pointer, leaving the wrapper with a freed buffer but a stale length and a dangling_pvt_ptr[0].<m>:memcpy'd through the freed/NULL buffer -> near-NULL segfault, andgetPtr()-then-CUDA read the dangling pointer -> use-after-free.Fix
Allocate and fill the new buffer first, and free/swap the old one only once the resize is known to succeed -- a strong exception guarantee. A failed (over)allocation now leaves the object completely unchanged.
Regeneration
The change is in the cybind generator template (
legacy_cython_gen); this PR is the full regenerated output of:Besides the setter fix (18
driver+ 11runtimesetters), the regen also picks up docstring-only cleanups that have landed in cybind since the last sync (e.g.nvml.pyx,runtime.pxd,cyruntime.pxd,runtime.rst). These are formatting-only -- no API, signature, or behavior changes -- and are included because they are part of what the canonical regeneration command produces.Verification
test_array_setter_preserves_state_on_failed_resizeforces the failed resize via an overflowing__len__(socallocoverflowssize_t-> NULL, no real host OOM needed). Passes; the two existing Fix a double-free bug #2112 array-setter regression tests still pass.Context
Addresses NVBUG 6268871. Companion cybind template change is under review separately; this PR exists so the regenerated bindings get full CI.