Uh oh!
There was an error while loading. Please reload this page.
Fix backport CI - #381
Conversation
leofang
commented
Jan 12, 2025
Let me add the backport label and admin-merge to test the backport CI. It should leave a comment in this PR to report an error. |
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 11.8.x
git worktree add -d .worktree/backport-381-to-11.8.x origin/11.8.x
cd .worktree/backport-381-to-11.8.x
git switch --create backport-381-to-11.8.x
git cherry-pick -x 6c3e074e9ff8776ad99b44def15bc22ec7adada2 |
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 11.8.x
git worktree add -d .worktree/backport-381-to-11.8.x origin/11.8.x
cd .worktree/backport-381-to-11.8.x
git switch --create backport-381-to-11.8.x
git cherry-pick -x 6c3e074e9ff8776ad99b44def15bc22ec7adada2 |
2 similar comments
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 11.8.x
git worktree add -d .worktree/backport-381-to-11.8.x origin/11.8.x
cd .worktree/backport-381-to-11.8.x
git switch --create backport-381-to-11.8.x
git cherry-pick -x 6c3e074e9ff8776ad99b44def15bc22ec7adada2 |
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin 11.8.x
git worktree add -d .worktree/backport-381-to-11.8.x origin/11.8.x
cd .worktree/backport-381-to-11.8.x
git switch --create backport-381-to-11.8.x
git cherry-pick -x 6c3e074e9ff8776ad99b44def15bc22ec7adada2 |
It didn't actually guard the two lifetime fixes: the driver copies the path during the set call (so the NVIDIA#379 latent UAF can't trigger), and a missing free leaks silently (so NVIDIA#381 wouldn't fail the test either). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…p helper lifetime (#2399) * Harden program-cache perms, binary-path resolution, and coredump helper lifetime - cuda.core: create the on-disk program cache tree owner-only (0o700) and re-assert restrictive perms on POSIX, so cached device code cannot be read or planted by other local users regardless of the inherited umask. - cuda.pathfinder: absolutize the resolved binary-utility path to honor the documented absolute-path contract; surface AddDllDirectory failures on Windows with GetLastError instead of silently swallowing them. - cuda.bindings: retain the caller's bytes in _HelperCUcoredumpSettings so the borrowed pointer cannot outlive its backing buffer, and free the getter's 1 KiB buffer in __dealloc__; clarify get_buffer_pointer's lifetime contract. Adds regression tests for the cache permissions, path absolutization, and coredump helper lifetime. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Simplify code comments on the hardening fixes Shorten the explanatory comments to a line or two each and drop the internal issue-number references (which don't resolve in this repo). No code changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Remove agent_authored markers from the new tests Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Remove weak coredump buffer-lifetime test It didn't actually guard the two lifetime fixes: the driver copies the path during the set call (so the #379 latent UAF can't trigger), and a missing free leaks silently (so #381 wouldn't fail the test either). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Silence bandit S103 on the intentionally world-writable test dir The test pre-creates a 0o777 cache dir to verify the loader tightens it to 0o700; the permissive mask is the point of the test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: expect absolutized binary path on Windows The abspath change makes find_nvidia_binary_utility return a drive-qualified path on Windows (C:\... ), so the three search-order tests must compare against os.path.abspath(expected). No-op on Linux; fixes the Windows CI failures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Raise on out-of-range c_int/c_byte kernel arguments instead of truncating The c_int/c_byte param feeders silently narrowed an out-of-range Python int (e.g. 2**32+5 -> 5). Range-check in the feeder and raise OverflowError; declare feed() as 'except? -1' so Cython propagates the exception instead of swallowing it. Addresses Glasswing V9.1 (leofang chose the raise option over matching ctypes' silent wrap). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * param_packer: use PyLong_AsInt for c_int range check on Python 3.13+ Per Leo's review: on 3.13+ PyLong_AsInt converts and range-checks in one call (raising OverflowError itself), so gate the manual INT_MIN/INT_MAX check to pre-3.13 only. c_byte keeps the manual check (no 8-bit CPython equivalent). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * param_packer: unify c_int/c_byte range check via PyLong_AsLongAndOverflow Per Leo's follow-up: use a single code path on all supported Pythons instead of version-gating PyLong_AsInt. PyLong_AsLongAndOverflow flags out-of-long values via its overflow out-param (no exception set), then we bounds-check the 32-bit (c_int) / 8-bit (c_byte) target range and raise OverflowError. Drops the PY_VERSION_HEX gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Split kernel-arg overflow change (#363) into its own PR (#2402) The out-of-range c_int/c_byte -> OverflowError change is a distinct behavior change; moved to a standalone PR so it can be reviewed separately from this hardening batch. No functional change to the remaining hardening work. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address review: scope cache perms to tmp, revert cybind docstring Per @leofang's review on PR #2399: - _file_stream.py: only the tmp/ staging dir is created owner-only (0o700). root/ and entries/ now inherit the umask / any pre-existing permissions so a deliberately shared kernel cache (e.g. group-writable on a compute cluster) keeps working. Dropped the post-mkdir chmod that would have re-tightened an existing shared directory. 0o700 in mkdir mode needs no chmod: umask only clears bits. - _internal/utils.pyx: revert the get_buffer_pointer docstring change; that is cybind code and is out of scope for this PR. - Tests updated to match: assert only tmp is 0o700, and that a pre-existing 0o777 shared root is used as-is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Document accepted security trade-off for shared program caches Record the residual risk from narrowing the cache-permission hardening (glasswing #359/#375) per PR #2399 review: committed entry files stay 0o600 (contents owner-only via mkstemp+os.replace), tmp/ is owner-only to protect in-flight writes, but root/entries inherit the umask so shared caches keep working. In a group-writable shared cache a group member could replace a cached kernel; tamper-proofing that path needs load-time integrity checks and is out of scope here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Simplify cache permission comment; drop internal tool name Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Strip internal issue numbers from cache permission test docstring Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * pathfinder: use os.add_dll_directory() instead of raw kernel32 call Per @leofang's review question: replace the direct ctypes kernel32.AddDllDirectory() call with the stdlib os.add_dll_directory() wrapper. Both call the same Win32 API, but the stdlib version drops the hand-maintained argtypes/restype binding and reports failure as OSError instead of a NULL cookie + GetLastError() check. Behavior is unchanged: the returned handle is intentionally discarded (it has no finalizer, so the directory stays on the search path for the process lifetime), and the PATH mutation + failure warning are preserved. Also strip an internal issue number from a test docstring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…p helper lifetime (NVIDIA#2399) * Harden program-cache perms, binary-path resolution, and coredump helper lifetime - cuda.core: create the on-disk program cache tree owner-only (0o700) and re-assert restrictive perms on POSIX, so cached device code cannot be read or planted by other local users regardless of the inherited umask. - cuda.pathfinder: absolutize the resolved binary-utility path to honor the documented absolute-path contract; surface AddDllDirectory failures on Windows with GetLastError instead of silently swallowing them. - cuda.bindings: retain the caller's bytes in _HelperCUcoredumpSettings so the borrowed pointer cannot outlive its backing buffer, and free the getter's 1 KiB buffer in __dealloc__; clarify get_buffer_pointer's lifetime contract. Adds regression tests for the cache permissions, path absolutization, and coredump helper lifetime. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Simplify code comments on the hardening fixes Shorten the explanatory comments to a line or two each and drop the internal issue-number references (which don't resolve in this repo). No code changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Remove agent_authored markers from the new tests Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Remove weak coredump buffer-lifetime test It didn't actually guard the two lifetime fixes: the driver copies the path during the set call (so the NVIDIA#379 latent UAF can't trigger), and a missing free leaks silently (so NVIDIA#381 wouldn't fail the test either). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Silence bandit S103 on the intentionally world-writable test dir The test pre-creates a 0o777 cache dir to verify the loader tightens it to 0o700; the permissive mask is the point of the test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: expect absolutized binary path on Windows The abspath change makes find_nvidia_binary_utility return a drive-qualified path on Windows (C:\... ), so the three search-order tests must compare against os.path.abspath(expected). No-op on Linux; fixes the Windows CI failures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Raise on out-of-range c_int/c_byte kernel arguments instead of truncating The c_int/c_byte param feeders silently narrowed an out-of-range Python int (e.g. 2**32+5 -> 5). Range-check in the feeder and raise OverflowError; declare feed() as 'except? -1' so Cython propagates the exception instead of swallowing it. Addresses Glasswing V9.1 (leofang chose the raise option over matching ctypes' silent wrap). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * param_packer: use PyLong_AsInt for c_int range check on Python 3.13+ Per Leo's review: on 3.13+ PyLong_AsInt converts and range-checks in one call (raising OverflowError itself), so gate the manual INT_MIN/INT_MAX check to pre-3.13 only. c_byte keeps the manual check (no 8-bit CPython equivalent). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * param_packer: unify c_int/c_byte range check via PyLong_AsLongAndOverflow Per Leo's follow-up: use a single code path on all supported Pythons instead of version-gating PyLong_AsInt. PyLong_AsLongAndOverflow flags out-of-long values via its overflow out-param (no exception set), then we bounds-check the 32-bit (c_int) / 8-bit (c_byte) target range and raise OverflowError. Drops the PY_VERSION_HEX gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Split kernel-arg overflow change (NVIDIA#363) into its own PR (NVIDIA#2402) The out-of-range c_int/c_byte -> OverflowError change is a distinct behavior change; moved to a standalone PR so it can be reviewed separately from this hardening batch. No functional change to the remaining hardening work. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address review: scope cache perms to tmp, revert cybind docstring Per @leofang's review on PR NVIDIA#2399: - _file_stream.py: only the tmp/ staging dir is created owner-only (0o700). root/ and entries/ now inherit the umask / any pre-existing permissions so a deliberately shared kernel cache (e.g. group-writable on a compute cluster) keeps working. Dropped the post-mkdir chmod that would have re-tightened an existing shared directory. 0o700 in mkdir mode needs no chmod: umask only clears bits. - _internal/utils.pyx: revert the get_buffer_pointer docstring change; that is cybind code and is out of scope for this PR. - Tests updated to match: assert only tmp is 0o700, and that a pre-existing 0o777 shared root is used as-is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Document accepted security trade-off for shared program caches Record the residual risk from narrowing the cache-permission hardening (glasswing NVIDIA#359/NVIDIA#375) per PR NVIDIA#2399 review: committed entry files stay 0o600 (contents owner-only via mkstemp+os.replace), tmp/ is owner-only to protect in-flight writes, but root/entries inherit the umask so shared caches keep working. In a group-writable shared cache a group member could replace a cached kernel; tamper-proofing that path needs load-time integrity checks and is out of scope here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Simplify cache permission comment; drop internal tool name Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Strip internal issue numbers from cache permission test docstring Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * pathfinder: use os.add_dll_directory() instead of raw kernel32 call Per @leofang's review question: replace the direct ctypes kernel32.AddDllDirectory() call with the stdlib os.add_dll_directory() wrapper. Both call the same Win32 API, but the stdlib version drops the hand-maintained argtypes/restype binding and reports failure as OSError instead of a NULL cookie + GetLastError() check. Behavior is unchanged: the returned handle is intentionally discarded (it has no finalizer, so the directory stays on the search path for the process lifetime), and the PATH mutation + failure warning are preserved. Also strip an internal issue number from a test docstring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…p helper lifetime (NVIDIA#2399) * Harden program-cache perms, binary-path resolution, and coredump helper lifetime - cuda.core: create the on-disk program cache tree owner-only (0o700) and re-assert restrictive perms on POSIX, so cached device code cannot be read or planted by other local users regardless of the inherited umask. - cuda.pathfinder: absolutize the resolved binary-utility path to honor the documented absolute-path contract; surface AddDllDirectory failures on Windows with GetLastError instead of silently swallowing them. - cuda.bindings: retain the caller's bytes in _HelperCUcoredumpSettings so the borrowed pointer cannot outlive its backing buffer, and free the getter's 1 KiB buffer in __dealloc__; clarify get_buffer_pointer's lifetime contract. Adds regression tests for the cache permissions, path absolutization, and coredump helper lifetime. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Simplify code comments on the hardening fixes Shorten the explanatory comments to a line or two each and drop the internal issue-number references (which don't resolve in this repo). No code changes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Remove agent_authored markers from the new tests Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Remove weak coredump buffer-lifetime test It didn't actually guard the two lifetime fixes: the driver copies the path during the set call (so the NVIDIA#379 latent UAF can't trigger), and a missing free leaks silently (so NVIDIA#381 wouldn't fail the test either). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Silence bandit S103 on the intentionally world-writable test dir The test pre-creates a 0o777 cache dir to verify the loader tightens it to 0o700; the permissive mask is the point of the test. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * test: expect absolutized binary path on Windows The abspath change makes find_nvidia_binary_utility return a drive-qualified path on Windows (C:\... ), so the three search-order tests must compare against os.path.abspath(expected). No-op on Linux; fixes the Windows CI failures. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Raise on out-of-range c_int/c_byte kernel arguments instead of truncating The c_int/c_byte param feeders silently narrowed an out-of-range Python int (e.g. 2**32+5 -> 5). Range-check in the feeder and raise OverflowError; declare feed() as 'except? -1' so Cython propagates the exception instead of swallowing it. Addresses Glasswing V9.1 (leofang chose the raise option over matching ctypes' silent wrap). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * param_packer: use PyLong_AsInt for c_int range check on Python 3.13+ Per Leo's review: on 3.13+ PyLong_AsInt converts and range-checks in one call (raising OverflowError itself), so gate the manual INT_MIN/INT_MAX check to pre-3.13 only. c_byte keeps the manual check (no 8-bit CPython equivalent). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * param_packer: unify c_int/c_byte range check via PyLong_AsLongAndOverflow Per Leo's follow-up: use a single code path on all supported Pythons instead of version-gating PyLong_AsInt. PyLong_AsLongAndOverflow flags out-of-long values via its overflow out-param (no exception set), then we bounds-check the 32-bit (c_int) / 8-bit (c_byte) target range and raise OverflowError. Drops the PY_VERSION_HEX gate. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Split kernel-arg overflow change (NVIDIA#363) into its own PR (NVIDIA#2402) The out-of-range c_int/c_byte -> OverflowError change is a distinct behavior change; moved to a standalone PR so it can be reviewed separately from this hardening batch. No functional change to the remaining hardening work. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Address review: scope cache perms to tmp, revert cybind docstring Per @leofang's review on PR NVIDIA#2399: - _file_stream.py: only the tmp/ staging dir is created owner-only (0o700). root/ and entries/ now inherit the umask / any pre-existing permissions so a deliberately shared kernel cache (e.g. group-writable on a compute cluster) keeps working. Dropped the post-mkdir chmod that would have re-tightened an existing shared directory. 0o700 in mkdir mode needs no chmod: umask only clears bits. - _internal/utils.pyx: revert the get_buffer_pointer docstring change; that is cybind code and is out of scope for this PR. - Tests updated to match: assert only tmp is 0o700, and that a pre-existing 0o777 shared root is used as-is. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Document accepted security trade-off for shared program caches Record the residual risk from narrowing the cache-permission hardening (glasswing NVIDIA#359/NVIDIA#375) per PR NVIDIA#2399 review: committed entry files stay 0o600 (contents owner-only via mkstemp+os.replace), tmp/ is owner-only to protect in-flight writes, but root/entries inherit the umask so shared caches keep working. In a group-writable shared cache a group member could replace a cached kernel; tamper-proofing that path needs load-time integrity checks and is out of scope here. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Simplify cache permission comment; drop internal tool name Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * Strip internal issue numbers from cache permission test docstring Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * pathfinder: use os.add_dll_directory() instead of raw kernel32 call Per @leofang's review question: replace the direct ctypes kernel32.AddDllDirectory() call with the stdlib os.add_dll_directory() wrapper. Both call the same Win32 API, but the stdlib version drops the hand-maintained argtypes/restype binding and reports failure as OSError instead of a NULL cookie + GetLastError() check. Behavior is unchanged: the returned handle is intentionally discarded (it has no finalizer, so the directory stays on the search path for the process lifetime), and the PATH mutation + failure warning are preserved. Also strip an internal issue number from a test docstring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
#379 (comment)