diff --git a/cuda_bindings/cuda/bindings/_lib/utils.pxd b/cuda_bindings/cuda/bindings/_lib/utils.pxd index 24b0ae8de93..0d8af74b4ff 100644 --- a/cuda_bindings/cuda/bindings/_lib/utils.pxd +++ b/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 diff --git a/cuda_bindings/cuda/bindings/_lib/utils.pxi b/cuda_bindings/cuda/bindings/_lib/utils.pxi index 2dd4d5c1a27..2796f910798 100644 --- a/cuda_bindings/cuda/bindings/_lib/utils.pxi +++ b/cuda_bindings/cuda/bindings/_lib/utils.pxi @@ -664,6 +664,8 @@ cdef class _HelperCUcoredumpSettings: self._cptr = 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 = 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): diff --git a/cuda_core/cuda/core/utils/_program_cache/_file_stream.py b/cuda_core/cuda/core/utils/_program_cache/_file_stream.py index 1e22a739ae0..eb71abf5446 100644 --- a/cuda_core/cuda/core/utils/_program_cache/_file_stream.py +++ b/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. diff --git a/cuda_core/tests/test_program_cache.py b/cuda_core/tests/test_program_cache.py index c305562dcba..a8d3fc85f7e 100644 --- a/cuda_core/tests/test_program_cache.py +++ b/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 diff --git a/cuda_pathfinder/cuda/pathfinder/_binaries/find_nvidia_binary_utility.py b/cuda_pathfinder/cuda/pathfinder/_binaries/find_nvidia_binary_utility.py index 4f857b9ac29..834db8fe8fa 100644 --- a/cuda_pathfinder/cuda/pathfinder/_binaries/find_nvidia_binary_utility.py +++ b/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 diff --git a/cuda_pathfinder/cuda/pathfinder/_dynamic_libs/load_dl_windows.py b/cuda_pathfinder/cuda/pathfinder/_dynamic_libs/load_dl_windows.py index 5069a624790..b2f61dfc9af 100644 --- a/cuda_pathfinder/cuda/pathfinder/_dynamic_libs/load_dl_windows.py +++ b/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") diff --git a/cuda_pathfinder/tests/test_find_nvidia_binaries.py b/cuda_pathfinder/tests/test_find_nvidia_binaries.py index ae439546859..2784633ff38 100644 --- a/cuda_pathfinder/tests/test_find_nvidia_binaries.py +++ b/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))