Skip to content

Unaligned safetensors reads need a checker: three recurrences found by UBSan, and one site UBSan cannot see #627

Description

@localai-bot

UBSan on the sanitize-cpu (address,undefined) CI leg:

src/vllm/model_executor/models/qwen3_5_weights.cpp:298:36: runtime error:
load of misaligned address 0x7f00df8027db for type const uint16_t,
which requires 2 byte alignment

TransposeBf16 (qwen3_5_weights.cpp:293-301) takes a const uint16_t* and dereferences it directly. One of its two callers, :334, produces that pointer by reinterpret_cast<const uint16_t*>(t.data), where t.data points into the mmapd safetensors buffer:

TransposeBf16(reinterpret_cast<const uint16_t*>(t.data), out_dim, in_dim,
              reinterpret_cast<uint16_t*>(o.bytes.data()));

Safetensors provides no alignment guarantee for tensor data offsets. They are byte offsets into a contiguous data block, determined by the sizes of preceding tensors — so a bf16 tensor that follows an odd-length tensor starts on an odd byte. Dereferencing a misaligned uint16_t* is undefined behavior regardless of whether x86 tolerates it in practice, and it is a real fault on stricter targets.

The other caller (:425) passes dq.data() from a std::vector<uint16_t>, which is suitably aligned. Only the :334 path is exposed.

Pre-existing, not introduced by the row that found it. git log -L 296,300 attributes the function to 8ee2c0766 ("Qwen3.6-35B MoE weight loader (NVFP4 dequant + stacked params + transpose)"). It surfaced now because #490s new test_qwen3_8_text_only builds synthetic checkpoints whose tensor offsets are not all even — which is a legitimate shape a real file can have.

Why no gate caught it before: the suites that exercise this path against real checkpoints are dgx-only and are not run under sanitizers, and the CPU sanitizer leg had no test that reached this loader until now. So the sanitizer lane and the loader coverage had never intersected.

Fix: load through std::memcpy (or char-wise) rather than dereferencing a cast pointer — the same treatment ReadScalarF32 at :288 already uses for exactly this reason, a few lines above. Worth grepping for sibling reinterpret_cast<... uint16_t*>(t.data) patterns in the loaders so the fix lands everywhere the assumption is made rather than only where UBSan happened to fire.

Any fix must prove inertness for the gated 27B/35B/Coder paths (golden md5 unchanged) since it touches the shared loader.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions