Skip to content

[Fix] model_loader: Avoid unsafe pickle fallback when loading PyTorch checkpoints - #42760

Merged
nvpohanh merged 14 commits into
sgl-project:mainfrom
nguyendvk:fix/torch-load-weights-only-upstream
Oct 27, 2026
Merged

nvpohanh merged 14 commits into
sgl-project:mainfrom
nguyendvk:fix/torch-load-weights-only-upstream

Conversation

@nguyendvk

@nguyendvk nguyendvk commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

torch.load(weights_only=False) deserializes with pickle, so a crafted checkpoint can execute code when it is loaded. _load_pt_file fell back to it for legacy tar checkpoints, and several loaders called torch.load on model files without weights_only=True. This is the same class of issue as #35653 (RIFE) and is consistent with the contribution guide's rule against unpickling untrusted data.

Modifications

  • Remove the weights_only=False fallback in _load_pt_file (srt/model_loader/weight_utils.py and multimodal_gen/runtime/loader/weight_utils.py). Legacy tar checkpoints now fail with torch's own error and should be converted to safetensors or a zip-format .pt offline.
  • Pass weights_only=True to torch.load in deepseek_ocr.py (x2), roberta.py and rife_interpolator.py.
  • Add unit tests for _load_pt_file: a tensor state dict still loads, and a pickle payload is not executed.
  • srt/debug_utils is unchanged: it reads developer-written tensor dumps, not model weights.

Accuracy Tests

Not applicable: only the checkpoint deserialization path changes; no model forward code is touched.

Speed Tests and Profiling

Not applicable.

Checklist

🤖 Generated with Claude Code


CI States

Latest PR Test (Base): ✅ Run #39535409763
Latest PR Test (Extra): ❌ Run #39535409518
Latest PR Test (AMD ROCm 10): ❌ Run #39535409559

nvdang and others added 2 commits October 6, 2026 13:11
Remove the weights_only=False fallback for legacy tar checkpoints and
pass weights_only=True to the remaining torch.load calls on model files,
so a malicious checkpoint cannot execute code via pickle. Legacy tar
checkpoints should be converted to safetensors offline.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@thanhhao98

Copy link
Copy Markdown
Contributor

/rerun-test test/registered/unit/model_loader/test_weight_utils.py python/sglang/multimodal_gen/test/unit/test_weight_utils.py test/registered/vlm/test_vision_openai_server_a.py::TestDeepseekOCRServer test/registered/unit/multimodal/test_deepseek_ocr_geometry.py python/sglang/multimodal_gen/test/server/test_server_1_gpu.py::TestDiffusionServerOneGpu::test_diffusion_generation[wan2_1_t2v_1.3b_frame_interp_2x]

@github-actions

github-actions Bot commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

🚀 ubuntu-latest (2 tests): ✅ View workflow run

cd test/ && python3 registered/unit/model_loader/test_weight_utils.py
cd test/ && python3 registered/unit/multimodal/test_deepseek_ocr_geometry.py

🚀 1-gpu-h100 (2 tests): ✅ View workflow run

python3 -m pytest python/sglang/multimodal_gen/test/unit/test_weight_utils.py -x
python3 -m pytest python/sglang/multimodal_gen/test/server/test_server_1_gpu.py::TestDiffusionServerOneGpu::test_diffusion_generation[wan2_1_t2v_1.3b_frame_interp_2x] -x

🚀 1-gpu-h100 (1 test): ✅ View workflow run

cd test/ && python3 registered/vlm/test_vision_openai_server_a.py TestDeepseekOCRServer

@thanhhao98

Copy link
Copy Markdown
Contributor

@nvpohanh pls help to review, thanks!

@nvpohanh nvpohanh left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[by Claude Code] Reviewed head 600dd5e. One inline comment on test coverage.

loaded = _load_pt_file(path)
self.assertTrue(torch.equal(loaded["w"], torch.arange(4)))

def test_pickle_payload_is_not_executed(self):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[by Claude Code] Severity: style | Confidence: High

Issue

test_pickle_payload_is_not_executed does not cover the removed fallback. torch.save writes a zip file, so torch.load(weights_only=True) raises UnpicklingError on main too, before the legacy .tar format branch runs. I ran TestLoadPtFile against main's _load_pt_file, and both tests pass.

Example

A legacy tar file with members storages (pickle.dumps(0) + pickle.dumps([])), tensors (pickle.dumps(0)), and pickle (the _OpenOnUnpickle payload) creates the marker on main and raises RuntimeError on this PR (checked with torch 2.14).

Fix

Build the payload as that legacy tar file with tarfile instead of torch.save, so the test fails if the fallback comes back.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks. Replaced the torch.save payload with a hand-built legacy tar checkpoint (storages/tensors/pickle members). Checked with torch 2.14: on main's _load_pt_file the payload runs and the marker is created; on this branch it raises RuntimeError and nothing runs. The tensor state-dict test is unchanged. Also thanks for the history on the fallback.

nvdang and others added 2 commits October 13, 2026 15:40
torch.save writes a zip file, so the previous payload test also passed
against the old fallback. Build a legacy tar checkpoint instead so the
test fails if the weights_only=False fallback is reintroduced.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@nvpohanh

Copy link
Copy Markdown
Collaborator

/tag-and-rerun-ci

@github-actions github-actions Bot added the run-ci CI: run the baseline test suite on this PR label Oct 13, 2026
@nvpohanh

Copy link
Copy Markdown
Collaborator

[by Claude Code] I've started babysitting the NVIDIA CI of this PR. I'll check it every 30 minutes until all NV pipelines pass. If a relevant job fails, I may rerun it, merge the latest main into this branch, push a small test-only fix, or post a note here.

Anyone can stop me at any time: just post a comment asking me to stop (for example, stop babysitting). I'll stop at my next check, within 30 minutes, and take no further action on this PR.

@nvpohanh

Copy link
Copy Markdown
Collaborator

[by Claude Code] The relevant CI failure (https://github.com/sgl-project/sglang/actions/runs/38384431344/job/115259669168) is not caused by this PR. It is a known breakage on main, tracked in #44054. Proposed fix: #44055. CI for this PR is blocked until that is fixed.

@nvpohanh

Copy link
Copy Markdown
Collaborator

[by Claude Code] I've started babysitting the NVIDIA CI of this PR. I'll check it every 30 minutes until all NV pipelines pass. If a relevant job fails, I may rerun it, merge the latest main into this branch, push a small test-only fix, or post a note here.

Anyone can stop me at any time: just post a comment asking me to stop (for example, stop babysitting). I'll stop at my next check, within 30 minutes, and take no further action on this PR.

@nvpohanh

Copy link
Copy Markdown
Collaborator

/rerun-failed-ci

…hts-only-upstream

# Conflicts:
#	test/registered/unit/model_loader/test_weight_utils.py
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@nvpohanh

Copy link
Copy Markdown
Collaborator

[by Claude Code] I've started babysitting the NVIDIA CI of this PR. I'll check it every 30 minutes until all NV pipelines pass. If a relevant job fails, I may rerun it, merge the latest main into this branch, push a small test-only fix, fix the PR title if the title check fails, or post a note here.

Anyone can stop me at any time: just post a comment asking me to stop (for example, stop babysitting). I'll stop at my next check, within 30 minutes, and take no further action on this PR.

@nvpohanh

Copy link
Copy Markdown
Collaborator

/rerun-failed-ci

@nvpohanh nvpohanh changed the title Avoid unsafe pickle fallback when loading PyTorch checkpoints [Fix] model_loader: Avoid unsafe pickle fallback when loading PyTorch checkpoints Oct 19, 2026
@nvpohanh

Copy link
Copy Markdown
Collaborator

[by Claude Code] @nguyendvk, the PR Title check failed, so I renamed this PR from Avoid unsafe pickle fallback when loading PyTorch checkpoints to [Fix] model_loader: Avoid unsafe pickle fallback when loading PyTorch checkpoints to match the required [Type] scope: title format. Feel free to change the type or scope if I picked the wrong one.

@nvpohanh

Copy link
Copy Markdown
Collaborator

/rerun-failed-ci

1 similar comment
@nvpohanh

Copy link
Copy Markdown
Collaborator

/rerun-failed-ci

Starts a fresh PR Test Base run: the GitHub jobs API keeps returning 502 for run 39239074876 (https://github.com/sgl-project/sglang/actions/runs/39239074876/job/118205011160), so rerunning that run cannot pass its wait-for-base-b gate.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@nvpohanh

Copy link
Copy Markdown
Collaborator

/rerun-failed-ci

1 similar comment
@nvpohanh

Copy link
Copy Markdown
Collaborator

/rerun-failed-ci

@nvpohanh

Copy link
Copy Markdown
Collaborator

[by Claude Code] The relevant CI failure (base-c-test-8-gpu-b300 (0)) is not caused by this PR. It is a known CI breakage from main that affects every PR from a fork, tracked in #45392: the gated Muse Spark test needs a repository secret that GitHub does not pass to fork PRs. Proposed fix: #45393. CI for this PR is blocked until that is fixed.

@nvpohanh

Copy link
Copy Markdown
Collaborator

[by Claude Code] I've started babysitting the NVIDIA CI of this PR. I'll check it every 30 minutes until all NV pipelines pass. If a relevant job fails, I may rerun it, merge the latest main into this branch, push a small test-only fix, fix the PR title if the title check fails, or post a note here.

Anyone can stop me at any time: just post a comment asking me to stop (for example, stop babysitting). I'll stop at my next check, within 30 minutes, and take no further action on this PR.

@nvpohanh

Copy link
Copy Markdown
Collaborator

[by Claude Code] The relevant CI failure (base-a-test-cpu (11)) is not caused by this PR. It is a known breakage on main that #45476 will fix. CI for this PR is blocked until that PR merges.

@nvpohanh

Copy link
Copy Markdown
Collaborator

/rerun-failed-ci

@nvpohanh

Copy link
Copy Markdown
Collaborator

[by Claude Code] All NV pipelines have passed.

@nvpohanh

Copy link
Copy Markdown
Collaborator

@mmangkad Could you review this? This is SGLang security issue. Thanks!

@nvpohanh

Copy link
Copy Markdown
Collaborator

@b8zhong @mickqian could you review this security change? Thanks!

@nvpohanh
nvpohanh merged commit f82f8a9 into sgl-project:main Oct 27, 2026
274 of 317 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

deepseek diffusion SGLang Diffusion run-ci CI: run the baseline test suite on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants