Repository navigation
qwen: refuse a PLE conv kernel the ring cannot hold - #1
Merged
Skibisky merged 1 commit intoSep 13, 2026
Merged
Conversation
A container with ple_conv_kernel_size = 0 declares no kernel, but two
defensive fallbacks ('> 0 ? value : 4') let it through the loader: the
weight check validates as if the kernel were 4 while the ring is
allocated (ple_conv_k - 1) * ngram_size = 0 elements, so the conv step
walks 4 taps across a zero-length ring — a heap-buffer-overflow READ
inside qwen_dilated_conv_step off m->ple_ring.
Refuse, don't normalize: ple_conv_k is now bounded 1..64 in cfg_sane(),
but only for containers that declare a PLE layer (ple_layer >= 0); the
field stays 0 and meaningless for every other architecture. Both
fallbacks are deleted so one stated meaning survives end to end:
the container's own declaration.
Verified on the synthetic Qwen container (make-test-container --qwen):
- base: test_forward with ple_conv_kernel_size 4 -> 0 aborts under
ASan+UBSan (heap-buffer-overflow READ in qwen_dilated_conv_step)
- fixed: the zero container is refused by the loader and waste info,
no sanitizer report
- valid control logits bit-for-bit identical before/after:
sha256 469a7433b7910b3f... both arms
- make asan suite: 76 passed / 0 failed / 17 skipped (base: 75/0/17);
the +1 is the new 'PLE conv kernel the ring cannot hold' refusal case
beside the existing layer_types refusals in tests/run.sh
Follow-up to the review discussion in sqliteai#63.
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.
Follow-up to the heap-buffer-overflow discussion in sqliteai#63 (marcobambini's ASan report, root-caused in my comment).
What this fixes
A container with
ple_conv_kernel_size = 0declares no kernel, but two defensive> 0 ? … : 4fallbacks let it through the loader:(ple_conv_k - 1) * ngram_size→ 0 elements;REQUIRE_DATAvalidates the conv weight as if the kernel were 4 and passes;KS = 4taps across a zero-length ring → heap-buffer-overflow READ insideqwen_dilated_conv_stepoffm->ple_ring.The fix — refuse, don't normalize
ple_conv_kis bounded 1..64 incfg_sane(), but only for containers that declare a PLE layer (ple_layer >= 0); the field stays 0 and meaningless for every other architecture, so kimi/GLM containers are untouched. Both> 0 ? … : 4fallbacks are deleted so one stated meaning survives end to end: the container's own declaration.Verified (ASan+UBSan, -fno-sanitize-recover=all, Apple M4)
c8040c6: synthetic Qwen container withple_conv_kernel_size4 → 0 aborts under ASan —heap-buffer-overflow READinqwen_dilated_conv_stepoffm->ple_ring.waste info(exit 1, no sanitizer report); the valid control runs unchanged.469a7433b7910b3f…identical before and after the fix.make asan76 passed / 0 failed / 17 skipped vs base 75/0/17 — the +1 is the newqwen_refused "a PLE conv kernel the ring cannot hold"case beside the existinglayer_typesrefusals intests/run.sh, so it runs in the normal suite.cc @Skibisky (you asked for this on sqliteai#63), @marcobambini