Skip to content

fix(runtime): validate routed-expert tensor shapes in Qwen GGUF loader#292

Open
RealDiligent wants to merge 1 commit into
gittensor-ai-lab:mainfrom
RealDiligent:fix/critical-issue-moe-expert-shape-validation
Open

fix(runtime): validate routed-expert tensor shapes in Qwen GGUF loader#292
RealDiligent wants to merge 1 commit into
gittensor-ai-lab:mainfrom
RealDiligent:fix/critical-issue-moe-expert-shape-validation

Conversation

@RealDiligent

Copy link
Copy Markdown

Fixes #291.

Root cause

Qwen35Model::load_gguf validates the shape of every tensor it loads with expect_dims — attention, router (ffn_gate_inp), shared experts (ffn_*_shexp), and norms — except the three routed-expert tensors, which were loaded with only a null-check:

w.gate_q = dev_quant(b + "ffn_gate_exps.weight", w.gate_qtype);   // no expect_dims
w.up_q   = dev_quant(b + "ffn_up_exps.weight",   w.up_qtype);
w.down_q = dev_quant(b + "ffn_down_exps.weight", w.down_qtype);

These are the largest, fully file-controlled weights, and their per-expert stride (hidden × moe_ffn) is assumed — never re-derived — by launch_moe_expert_ffn_q4k (called with c.hidden / c.moe_ffn from the GGUF metadata). A GGUF whose expert_count / expert_feed_forward_length metadata disagrees with the actual tensor extent therefore loads cleanly, then the expert FFN computes offsets expert_id * hidden * moe_ffn with the wrong stride and reads out of bounds of the quantized blob → corrupt routed output or an illegal-address crash.

Fix

Add the missing expect_dims checks before the dev_quant loads, mirroring the existing ffn_*_shexp validation with the expert axis appended (ggml ne order):

tensor expected dims
ffn_gate_exps.weight {H, moe_ffn, n_experts}
ffn_up_exps.weight {H, moe_ffn, n_experts}
ffn_down_exps.weight {moe_ffn, H, n_experts}

A shape-mismatched GGUF is now rejected up front with a clear [gguf] bad shape … error, before any device pointer arithmetic runs.

Impact / risk

  • Backward compatible. Valid Qwen3-MoE ({2048, 768, 128}) and Qwen3.6-35B-A3B ({2048, 512, 256}) GGUFs load unchanged — the expert tensors already carry exactly these extents (the runtime relies on them today).
  • 10-line change, one file, host-side only; no kernel or ABI change.
  • The dimension ordering was cross-checked against the sibling ffn_*_shexp checks in the same function, the transposes in runtime/tools/convert_gguf.py, and the standard llama.cpp MoE ne layout.

Verification

No GPU/CUDA build was available in this environment, so the dimension-ordering logic was verified by re-implementing expect_dims against the real tensor layouts:

  • valid Qwen3.6 and Qwen3-30B expert shapes → accepted (models still load);
  • wrong expert_count, wrong moe_ffn, transposed down axes, and 2-D rank → rejected before the kernel runs.

@skyrocket2026 skyrocket2026 added area:runtime subsystem (emission weight 0.26) not-tested Awaiting maintainer approval to run on RTX 5090; not evaluated labels Jul 9, 2026
@RealDiligent RealDiligent force-pushed the fix/critical-issue-moe-expert-shape-validation branch from e62cc24 to bff3d84 Compare July 10, 2026 08:04
load_gguf validates the shape of every attention, router, shared-expert,
and norm tensor with expect_dims, but loads the three routed-expert
tensors (ffn_gate_exps / ffn_up_exps / ffn_down_exps) with only a
null-check. These are the largest, fully file-controlled weights, and
their per-expert stride (hidden x moe_ffn) is assumed — never
re-derived — by launch_moe_expert_ffn_q4k.

A GGUF whose expert_count / expert_feed_forward_length metadata disagrees
with the actual tensor extent therefore loads cleanly, then the expert
FFN computes expert base offsets expert_id * hidden * moe_ffn with the
wrong stride and reads out of bounds of the quantized blob — corrupt
routed output or an illegal-address crash, all from a file-controlled
dimension.

Add the missing expect_dims checks (ggml ne order: gate/up
{H, moe_ffn, n_experts}, down {moe_ffn, H, n_experts}) so a mismatched
GGUF is rejected up front with a clear error, mirroring the existing
ffn_*_shexp validation. Valid Qwen3-MoE and Qwen3.6 GGUFs are unaffected.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@skyrocket2026 skyrocket2026 added the needs-rebase Verified speedup but not the round winner — rebase after merge-first lands label Jul 11, 2026
@skyrocket2026

Copy link
Copy Markdown
Member

⏸ Merge conflict — rebase before eval

This branch conflicts with main, so the RTX 5090 eval is skipped until you rebase.

Please rebase onto main and push — the bot re-evaluates on the next poll once the PR is cleanly mergeable.

@skyrocket2026

Copy link
Copy Markdown
Member

The round's merge-first PR was just merged. Please rebase this branch onto main — once you push the rebase the bot re-evaluates it against the new frontier (crediting your marginal gain on top of what merged).

3 similar comments
@skyrocket2026

Copy link
Copy Markdown
Member

The round's merge-first PR was just merged. Please rebase this branch onto main — once you push the rebase the bot re-evaluates it against the new frontier (crediting your marginal gain on top of what merged).

@skyrocket2026

Copy link
Copy Markdown
Member

The round's merge-first PR was just merged. Please rebase this branch onto main — once you push the rebase the bot re-evaluates it against the new frontier (crediting your marginal gain on top of what merged).

@skyrocket2026

Copy link
Copy Markdown
Member

The round's merge-first PR was just merged. Please rebase this branch onto main — once you push the rebase the bot re-evaluates it against the new frontier (crediting your marginal gain on top of what merged).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:runtime subsystem (emission weight 0.26) needs-rebase Verified speedup but not the round winner — rebase after merge-first lands not-tested Awaiting maintainer approval to run on RTX 5090; not evaluated

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix(runtime): load_gguf skips shape validation on the routed-expert tensors — file-controlled OOB read in the MoE expert FFN

3 participants