Skip to content

Fix 5 crash bugs: stack overflow, NULL deref, unchecked mallocs in glm.c#183

Merged
JustVugg merged 1 commit into
JustVugg:devfrom
woolcoxm:fix/c-engine-crash-bugs
Jul 14, 2026
Merged

Fix 5 crash bugs: stack overflow, NULL deref, unchecked mallocs in glm.c#183
JustVugg merged 1 commit into
JustVugg:devfrom
woolcoxm:fix/c-engine-crash-bugs

Conversation

@woolcoxm

Copy link
Copy Markdown
Contributor

Problem

Static analysis of glm.c found five crash-class bugs — stack buffer overflows, NULL pointer dereferences, and unchecked mallocs — all in code paths that run at startup or during serve. Each can crash the engine; none affect output correctness (they crash, they don't silently corrupt).

Bugs fixed

C1: Stack buffer overflow in forward_all (line ~2709)

// Before:
float row[8192]; rmsnorm(row, x+(int64_t)s*D, m->final_norm, D, c->eps);
// After:
float *row=falloc(D); rmsnorm(row, x+(int64_t)s*D, m->final_norm, D, c->eps);

Why: The config checker (CKR at line ~955) allows hidden_size up to 1<<20, but the fixed stack buffer holds only 8192 floats (32 KB). Any model with hidden > 8192 overflows the stack. Every other hot path in the file correctly uses falloc(D) for this row — this was the one outlier. GLM-5.2 (D=6144) is safe today, but this is a latent crash for larger configs.

C2: NULL dereference in read_arr (line ~3334)

// Before:
static int *read_arr(jval*o,const char*k,int*n){ jval*a=json_get(o,k); int*r=malloc(a->len*sizeof(int)); ...
// After:
jval*a=json_get(o,k); if(!a){ *n=0; return NULL; } ...

Why: json_get returns NULL when the key is absent. The code immediately dereferences a->len — guaranteed segfault. Called from main() when loading ref_glm.json for oracle validation. If the file is missing prompt_ids or full_ids, the engine crashes instead of reporting the error. Added a NULL check + the caller now validates the result.

C3: Unchecked malloc in mux_submit (line ~3103)

// Before:
int *tmp=malloc(maxctx*sizeof(int));
int nt=tok_encode(T,raw,(int)sub.bytes,tmp,maxctx-2);
// After:
int *tmp=malloc(maxctx*sizeof(int));
if(!tmp){ fprintf(stderr,"OOM mux_submit tmp\n"); free(raw); free(line); exit(1); }

Why: tmp is used immediately without a NULL check. The sibling allocations in the same function (raw at ~3260, tmp at ~3275) both check for NULL — this one was missed. On a 744B model, OOM is a real scenario.

C4: Unchecked malloc for s->hist in serve_ctx_init (line ~3025)

// Before:
s->hist=malloc(maxctx*sizeof(int)); s->first=1;
// After:
s->hist=malloc(maxctx*sizeof(int));
if(!s->hist){ fprintf(stderr,"OOM serve_ctx_init hist\n"); exit(1); }
s->first=1;

Why: s->hist is passed to kv_disk_load which writes token IDs into it. NULL here writes through a null pointer. The adjacent kv_alloc uses falloc which aborts cleanly; this path silently returns NULL.

C5: Stack buffer overflow in rope_interleave (line ~896)

// Before:
int half = c->qk_rope/2; float in[256]; memcpy(in,v,c->qk_rope*sizeof(float));
// After:
if(c->qk_rope > 256){ fprintf(stderr,"qk_rope=%d exceeds rope_interleave buffer (256)\n",c->qk_rope); exit(1); }

Why: The config checker allows qk_rope up to 1<<16, but the local buffer holds 256 floats. A config with qk_rope > 256 corrupts the stack. GLM-5.2 (qk_rope=64) is safe today. Added a runtime bounds check that fails cleanly instead of silently smashing the stack.

Verification

  • Builds clean: make glm.exe ARCH=native (exit 0)
  • All 7 C test suites pass (make test-c)
  • Smoke test: SNAP=glm52_i4 NGEN=2 PROMPT="hi" ./glm.exe 8 loads and runs correctly

Files changed

  • c/glm.c — 5 locations (20 insertions, 6 deletions)

Based on dev (6d3ed7e).

Five crash-class bugs found by static analysis, all in glm.c:

C1: forward_all (line ~2709) used a fixed stack buffer float row[8192] for
    the final RMS norm. The config checker allows hidden_size up to 1<<20;
    any model with hidden > 8192 would smash the stack. Every other hot path
    in the file correctly uses falloc(D). Replaced with falloc(D) + free.

C2: read_arr (line ~3334) dereferenced json_get() return without a NULL
    check. If ref_glm.json is missing 'prompt_ids' or 'full_ids', this is a
    guaranteed NULL-pointer dereference / segfault. Added a NULL check that
    returns NULL+0, and the caller in main() now validates the result.

C3: mux_submit (line ~3103) allocated tmp=malloc(maxctx*sizeof(int)) without
    a NULL check, then passed it to tok_encode. OOM here writes through NULL.
    Added a check matching the sibling allocations in the same function.

C4: serve_ctx_init (line ~3025) allocated s->hist without a NULL check, then
    passed it to kv_disk_load which writes token IDs into it. Added a check
    matching the falloc() pattern used by kv_alloc.

C5: rope_interleave (line ~896) used a fixed stack buffer float in[256] but
    the config checker allows qk_rope up to 1<<16. Added a runtime bounds
    check that exits with a clear message instead of silently overflowing.
    GLM-5.2 qk_rope=64, well within bounds.
@JustVugg
JustVugg merged commit d872dd5 into JustVugg:dev Jul 14, 2026
JustVugg added a commit to dnnspaul/colibri that referenced this pull request Jul 14, 2026
JustVugg added a commit to ZacharyZcR/colibri that referenced this pull request Jul 14, 2026
maxwell1500 added a commit to maxwell1500/colibri that referenced this pull request Jul 14, 2026
…locs, null derefs (JustVugg#183)  Five crash-class bugs found by static analysis, all in glm.c:  C1: forward_all (line ~2709) used a fixed stack buffer float row[8192] for     the final RMS norm. The config checker allows hidden_size up to 1<<20;     any model with hidden > 8192 would smash the stack. Every other hot path     in the file correctly uses falloc(D). Replaced with falloc(D) + free.  C2: read_arr (line ~3334) dereferenced json_get() return without a NULL     check. If ref_glm.json is missing 'prompt_ids' or 'full_ids', this is a     guaranteed NULL-pointer dereference / segfault. Added a NULL check that     returns NULL+0, and the caller in main() now validates the result.  C3: mux_submit (line ~3103) allocated tmp=malloc(maxctx*sizeof(int)) without     a NULL check, then passed it to tok_encode. OOM here writes through NULL.     Added a check matching the sibling allocations in the same function.  C4: serve_ctx_init (line ~3025) allocated s->hist without a NULL check, then     passed it to kv_disk_load which writes token IDs into it. Added a check     matching the falloc() pattern used by kv_alloc.  C5: rope_interleave (line ~896) used a fixed stack buffer float in[256] but     the config checker allows qk_rope up to 1<<16. Added a runtime bounds     check that exits with a clear message instead of silently overflowing.     GLM-5.2 qk_rope=64, well within bounds.  Co-authored-by: woolcoxm <13604288+woolcoxm@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants