Repository navigation
fix(security): check the CUDA PTX cache directory on Windows - #2799
Merged
Merged
Conversation
The _WIN32 branches of jit_module.cpp loaded modules from a cache directory they never checked. The overlay now applies the owner and DACL rules of the ROCm cache to MLX_PTX_CACHE_DIR, the persistent default and the temp fallback, and refuses a relative MLX_PTX_CACHE_DIR or MLXCEL_CACHE_DIR on all platforms. On Windows the persistent default had no effect: Rust's set_var does not reach the C runtime's getenv, so entries went to %TEMP%\mlx and cuda-ptx/<commit> stayed empty. The overlay now reads the process environment, so the first run after an upgrade compiles once. make_private_dir clears group write only, leaving other write for the overlay to refuse. cache_root treats an empty MLXCEL_CACHE_DIR as unset. Validated on the Windows N1X: the new test fails 15 of 19 on the old overlay and passes 19 of 19 here, and real mlxcel and mlxcel-server runs refuse a shared directory and hit on the second run in a private one. The POSIX branches and Linux tests did not run on this host. Closes #2797
Review of #2799 found that a directory shared and later made private still loaded what was planted in it. On CUDA and ROCm each entry file now passes the owner and write rules on load, through one handle opened without following a link. A refusal is a miss that recompiles and republishes. The rule takes the owner and ACL as input, so a foreign owner, a null DACL and unmapped generic rights are tested without a second account or FAT. FILE_WRITE_ATTRIBUTES joins the write mask in both headers, and TrustedInstaller is trusted like SYSTEM, since it owns C:\Windows, where service profiles live. An empty MLX_PTX_CACHE_DIR now counts as unset. CI: overlay-host-tests-windows runs both host tests with real icacls on windows-latest, without a GPU, and cpu-link now runs the ptx_cache_dirs tests. Validated on the N1X: GPU test 22 of 22, both host tests pass, and 27 of 29 mutants fail the host tests. The two reparse-attribute mutants survive here because this host cannot create a file symlink; that case needs the elevated CI runner. No process ran as SYSTEM or NetworkService. Refs #2797
inureyes
added a commit
that referenced
this pull request
Nov 1, 2026
On Windows `std::env::set_var` does not update the C runtime's `getenv`, which MLX reads, so every default mlxcel writes for MLX was inert there. `mlxcel_core::native_env` also calls `_wputenv_s`, in the one UCRT that Rust, the bridge, MLX and the nvcc objects share (all `MD_DynamicRelease`, measured). Production writes go through it, and `scripts/ci/check_native_env_writes.py` fails CI on a direct one. Other platforms still only make the `std::env` call. ## Audit | Variable | Writer | Native reader | Effective on Windows before | |---|---|---|---| | `MLX_USE_CUDA_GRAPHS` | `loading/mod.rs` | `device.cpp` | No, capture stayed on for Gemma 4 | | `MLX_CUDA_GRAPH_CACHE_SIZE` | `hardware.rs` | `lru_cache.h` | No, 400 | | `MLX_CUDA_SDPA_CACHE_SIZE` | `hardware.rs` | `lru_cache.h` | No, 256 | | `MLX_MAX_OPS_PER_BUFFER` | `cuda_graph_budget.rs`, `hardware.rs` (Metal) | `get_graph_limits` | No, 20 for Laguna on 12.1 | | `MLX_MAX_MB_PER_BUFFER` | `cuda_graph_budget.rs` | `get_graph_limits` | No, 25 for Laguna on 12.1 | | `MLX_PTX_CACHE_DIR` | `ensure_persistent_ptx_cache` | `ptx_cache_dir_win.h` (wide) | Yes, since #2799 | | `MLXCEL_QMM_TILE_M` | `cuda_kernel_knobs.rs` | `qmm_sm80.cu` | No | | `MLXCEL_QMV_MULTIROW_MAX_ROWS` | `cuda_kernel_knobs.rs` | `qmv.cu` | No | `MLXCEL_KV_BOUNDARY_V_LAYERS` and `MLXCEL_PREFILL_CHUNK` are read by Rust only; they use the helper so the guard has one rule. `MLX_ENABLE_TF32` and the `mlx_server.rs` writes are test-only. ## Behavior change on Windows Gemma 4 runs with graph capture off, Laguna gets the 100 ops and 1000 "MB" budget on compute capability 12.1, and both cache capacities are 2000. The N1X serving A/B is run separately by the owner; no serving benchmark ran here. ## Validation On an RTX Spark N1X: 5 of 8 `native_env` tests fail without the `_wputenv_s` call and 8 of 8 pass with it, reading through the bridge and `mlx::core::env::get_var`. `cuda_ptx_cache_trust_windows` passes 22 of 22. `RUST_LOG=mlxcel_core::native_env=debug` showed the expected values in `mlxcel` and `mlxcel-server` for config-only llama, gemma4 and laguna directories, and an operator value still wins. Not run: POSIX locally (CI covers it), weights on these paths, and a cache grown past 400 entries. Closes #2803
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 free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
The
_WIN32branches ofjit_module.cpploaded modules from a cache directory they never checked. Per item in #2797:ptx_cache_dir_win.handptx_cache_acl_win.happly the rules ofhsaco_cache_dir_win.htoMLX_PTX_CACHE_DIR, the persistent default, the temp fallback and the device subdirectory. The walk removes the\\?\prefix, becausestd::filesystemotherwise parses\\?as the root and walks past the drive; file system calls add it back, so paths pastMAX_PATHstill work.MLX_PTX_CACHE_DIRorMLXCEL_CACHE_DIRis refused on all platforms, andensure_persistent_ptx_cacheno longer creates directories under the current directory.make_private_dirclears group write only, and other write stays for the overlay to refuse.cache_roottreats an emptyMLXCEL_CACHE_DIRas unset.From review, on CUDA and ROCm unless noted:
FILE_WRITE_ATTRIBUTESjoins the write mask, and TrustedInstaller is trusted like SYSTEM, so a service profile underC:\Windowscan cache.MLX_PTX_CACHE_DIRcounts as unset (CUDA).Behavior changes on Windows, not in the issue. The persistent default never took effect: Rust's
set_vardoes not reach the C runtime'sgetenv, so entries went to%TEMP%\mlxandcuda-ptx/<commit>stayed empty, measured with themainbinary. The overlay now reads the process environment, somlxcelandmlxcel-serveruse the documented directory and compile once after upgrading. Anyone who setMLX_PTX_CACHE_DIR=to turn the cache off now gets the default directory.Validation
On the Windows N1X with real
icaclsACLs:tests/cuda_ptx_cache_trust_windows.rs: 15 of the first 19 cases failed on the old overlay, and all 22 pass now. Planted entries load from a private directory (the control), and do not load from a shared one or with a foreign write entry on the file.mlxcel generate: a default directory compiles, then hits. A root inheriting Authenticated Users write prints... is writable by S-1-5-11and compiles with warm entries planted. A grant on one entry recompiles, then hits. A relative root is refused and nothing is created.mlxcel-server(before the review fixes): a shared root is refused. A private root compiles, then hits.In CI:
overlay-host-tests-windowsruns both host tests onwindows-latestwithout a GPU, andcpu-linknow runs theptx_cache_dirstests.Did not run: Linux GPU tests in
entries.rs, which need GB10. The two reparse-attribute mutants survive locally, because this host cannot create a file symlink, so that case runs only on the elevated CI runner and was not tried against a mutant. Item 4's test was not run against the old code. No process ran as SYSTEM or NetworkService, and a real second drive was not exercised.Closes #2797