Harden program-cache permissions, binary-path resolution, and coredump 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>

Author: rparolin

cuda_bindings/cuda/bindings/_lib/utils.pxd

@@ -162,6 +162,7 @@ cdef class _HelperCUcoredumpSettings:
     cdef cydriver.CUcoredumpSettings_enum _attrib
     cdef bint _is_getter
     cdef size_t _size
+    cdef object _references  # keeps caller bytes alive so _charstar stays valid
 
     # Return values
     cdef bint _bool

cuda_bindings/cuda/bindings/_lib/utils.pxi

@@ -664,6 +664,8 @@ cdef class _HelperCUcoredumpSettings:
                 self._cptr = <void*><void_ptr>self._charstar
                 self._size = 1024
             else:
+                # Keep a reference so the borrowed _charstar buffer stays alive.
+                self._references = init_value
                 self._charstar = init_value
                 self._cptr = <void*><void_ptr>self._charstar
                 self._size = len(init_value)
@@ -680,7 +682,11 @@ cdef class _HelperCUcoredumpSettings:
             raise TypeError('Unsupported attribute: {}'.format(attr.name))
 
     def __dealloc__(self):
-        pass
+        # Only the getter path owns heap (the calloc'd 1024-byte buffer). The
+        # setter borrows caller bytes and the bool path points at &self._bool,
+        # so only free for the getter.
+        if self._is_getter:
+            free(self._charstar)
 
     @property
     def cptr(self):

cuda_core/cuda/core/utils/_program_cache/_file_stream.py

@@ -397,9 +397,19 @@ def __init__(
         self._entries = self._root / _ENTRIES_SUBDIR
         self._tmp = self._root / _TMP_SUBDIR
         self._max_size_bytes = max_size_bytes
+        # Permissions (see PR #2399):
+        # root/ and entries/ use default permissions so a shared cache (e.g. one
+        # a group shares on a cluster) keeps working. The cached files themselves
+        # are still private: each is written to tmp/ as owner-only and moved into
+        # entries/, which keeps its permissions. tmp/ is made owner-only so no one
+        # can read or swap a file while it's being written. We don't chmod, so an
+        # existing directory is left as-is.
+        # Trade-off: if a group deliberately shares a writable entries/, a member
+        # could replace a cached file. Blocking that needs a check at load time,
+        # not just permissions, and is out of scope here.
         self._root.mkdir(parents=True, exist_ok=True)
         self._entries.mkdir(exist_ok=True)
-        self._tmp.mkdir(exist_ok=True)
+        self._tmp.mkdir(exist_ok=True, mode=0o700)
         # Opportunistic startup sweep of orphaned temp files left by any
         # crashed writers. Age-based so concurrent in-flight writes from
         # other processes are preserved.

cuda_core/tests/test_program_cache.py

@@ -3,6 +3,7 @@
 # SPDX-License-Identifier: Apache-2.0
 
 import abc
+import os
 import time
 
 import pytest
@@ -2535,3 +2536,40 @@ def reader(tid: int) -> None:
     # Internal accounting must agree with the cap and with __len__.
     assert cache._total_bytes <= 4096
     assert len(cache) == len(cache._entries)  # no orphan entries
+
+
+@pytest.mark.skipif(os.name == "nt", reason="POSIX permission bits only")
+def test_program_cache_tmp_dir_created_owner_only(tmp_path):
+    """``tmp`` stages in-flight compiled device code before the atomic rename into
+    ``entries``, so it must be created owner-only (0o700) regardless of the inherited
+    umask. ``root``/``entries`` intentionally inherit the umask to keep deliberately
+    shared caches working (PR #2399 review), so only ``tmp`` is asserted."""
+    import stat
+
+    from cuda.core.utils._program_cache._file_stream import FileStreamProgramCache
+
+    root = tmp_path / "pc"
+    FileStreamProgramCache(path=root)
+
+    mode = stat.S_IMODE(os.stat(root / "tmp").st_mode)
+    assert mode == 0o700, f"tmp has mode {oct(mode)}"
+
+
+@pytest.mark.skipif(os.name == "nt", reason="POSIX permission bits only")
+def test_program_cache_preexisting_shared_root_used_as_is(tmp_path):
+    """PR #2399 review: a deliberately shared cache root (e.g. group-writable on a
+    compute cluster) must be used as-is, not re-tightened. Only ``tmp`` is forced
+    owner-only; ``root`` keeps whatever permissions it was created with."""
+    import stat
+
+    from cuda.core.utils._program_cache._file_stream import FileStreamProgramCache
+
+    root = tmp_path / "pc"
+    root.mkdir()
+    # Simulate an intentionally shared cache directory.
+    os.chmod(root, 0o777)  # noqa: S103
+
+    FileStreamProgramCache(path=root)
+
+    assert stat.S_IMODE(os.stat(root).st_mode) == 0o777
+    assert stat.S_IMODE(os.stat(root / "tmp").st_mode) == 0o700

cuda_pathfinder/cuda/pathfinder/_binaries/find_nvidia_binary_utility.py

@@ -63,7 +63,9 @@ def _resolve_in_trusted_dirs(normalized_name: str, dirs: list[str]) -> str | Non
         seen.add(directory)
         candidate = os.path.join(directory, normalized_name)
         if _is_executable_candidate(candidate):
-            return candidate
+            # Return an absolute path, as the docstring promises (a relative
+            # search dir would otherwise leak a relative result).
+            return os.path.abspath(candidate)
     return None
 
 

cuda_pathfinder/cuda/pathfinder/_dynamic_libs/load_dl_windows.py

@@ -7,6 +7,7 @@
 import ctypes.wintypes
 import os
 import struct
+import warnings
 from typing import TYPE_CHECKING
 
 from cuda.pathfinder._dynamic_libs.load_dl_common import LoadedDL
@@ -43,10 +44,6 @@
 ]
 kernel32.GetModuleFileNameW.restype = ctypes.wintypes.DWORD
 
-# AddDllDirectory (Windows 7+)
-kernel32.AddDllDirectory.argtypes = [ctypes.wintypes.LPCWSTR]
-kernel32.AddDllDirectory.restype = ctypes.c_void_p  # DLL_DIRECTORY_COOKIE
-
 
 def ctypes_handle_to_unsigned_int(handle: ctypes.wintypes.HMODULE) -> int:
     """Convert ctypes HMODULE to unsigned int."""
@@ -69,10 +66,23 @@ def add_dll_directory(dll_abs_path: str) -> None:
     dirpath = os.path.dirname(dll_abs_path)
     assert os.path.isdir(dirpath), dll_abs_path
 
-    # Add the DLL directory to the native search path. AddDllDirectory only
-    # affects the LOAD_LIBRARY_SEARCH_USER_DIRS search; PATH is updated
-    # unconditionally below to also cover legacy dependent-DLL resolution.
-    kernel32.AddDllDirectory(dirpath)
+    # Add the DLL directory to the native search path via the stdlib wrapper
+    # around AddDllDirectory. This only affects the LOAD_LIBRARY_SEARCH_USER_DIRS
+    # search; PATH is updated unconditionally below to also cover legacy
+    # dependent-DLL resolution. The returned handle is intentionally discarded:
+    # the directory must stay on the search path for the process lifetime, and
+    # the handle has no finalizer, so dropping it does not remove the directory.
+    try:
+        os.add_dll_directory(dirpath)  # type: ignore[attr-defined]
+    except OSError as e:
+        # Warn instead of failing silently; the PATH update below is a weaker
+        # fallback that newer loaders may ignore.
+        warnings.warn(
+            f"os.add_dll_directory({dirpath!r}) failed ({e}); "
+            "falling back to process-global PATH mutation for dependent-DLL resolution.",
+            RuntimeWarning,
+            stacklevel=2,
+        )
 
     # Update PATH as a fallback for dependent DLL resolution
     curr_path = os.environ.get("PATH")

cuda_pathfinder/tests/test_find_nvidia_binaries.py

@@ -152,7 +152,7 @@ def test_find_binary_first_matching_dir_wins(monkeypatch, mocker):
     result = find_nvidia_binary_utility("nvcc")
 
     # Conda comes before CUDA_HOME, so the Conda hit wins and CUDA_HOME is never probed.
-    assert result == conda_nvcc
+    assert result == os.path.abspath(conda_nvcc)
     assert checked == [os.path.join(site_dir, "nvcc"), conda_nvcc]
 
 
@@ -173,7 +173,7 @@ def test_find_binary_ctk_root_canary_fallback(monkeypatch, mocker):
 
     result = find_nvidia_binary_utility("nvcc")
 
-    assert result == ctk_nvcc
+    assert result == os.path.abspath(ctk_nvcc)
     canary_mock.assert_called_once_with()
     # No earlier trusted dirs existed, so the only probe is the canary bin dir.
     assert checked == [ctk_nvcc]
@@ -218,7 +218,7 @@ def test_find_binary_canary_not_consulted_when_found_earlier(monkeypatch, mocker
 
     result = find_nvidia_binary_utility("nvcc")
 
-    assert result == conda_nvcc
+    assert result == os.path.abspath(conda_nvcc)
     canary_mock.assert_not_called()
 
 
@@ -370,3 +370,27 @@ def test_caching_per_utility():
     # them is None)
     if nvdisasm1 is not None and nvcc1 is not None:
         assert nvdisasm1 != nvcc1
+
+
+def test_resolve_in_trusted_dirs_returns_absolute_path(tmp_path, monkeypatch, mocker):
+    """A match found under a relative search dir must be absolutized.
+
+    ``find_nvidia_binary_utility`` documents an absolute, separator-resolved
+    result. A relative search dir (e.g. a relative ``CUDA_HOME``) previously
+    leaked a relative path that would re-resolve against a possibly different
+    CWD at execution time.
+    """
+    rel_dir = os.path.join("some", "relative", "bin")
+    candidate = os.path.join(rel_dir, "nvcc")
+    mocker.patch.object(
+        binary_finder_module,
+        "_is_executable_candidate",
+        side_effect=lambda path: path == candidate,
+    )
+
+    # Anchor CWD so os.path.abspath is deterministic for the assertion.
+    monkeypatch.chdir(tmp_path)
+    result = binary_finder_module._resolve_in_trusted_dirs("nvcc", [rel_dir])
+
+    assert os.path.isabs(result)
+    assert result == os.path.abspath(os.path.join(str(tmp_path), candidate))