Heap buffer overrun in stepcompress.c queue_append_far() when the step queue is full (batch mode)

Developers
1

Hi,

I would like to report a heap buffer overrun in klippy/chelper/stepcompress.c, reproducible on unmodified upstream master (v0.13.0-820-g0cee841e3) in batch mode.

Background

We build the RF50, a Klipper-based 3D printer, and test configuration changes by running full prints through batch mode in CI. A configuration with an extruder stepper synced to the extruder started crashing with SIGSEGV mid-print. I reproduced it on plain upstream with a standard [extruder_stepper]; the attached files come from that reproduction.

The bug

In queue_append_far(), when the step is not far enough out for stepcompress_flush_far(), it is stored with sc->queue_next->clock32 = step_clock; sc->queue_next++; and there is no queue_next >= queue_end check. queue_append() tests for a far step before it tests for a full queue, so a full queue plus a far step writes one slot past the end of the buffer.

In batch mode, _flush_handler_debug() can make a single step generation call cover tens of seconds (25.5 s in this reproduction). Once a stepper has CLOCK_DIFF_MAX of pending steps, every append takes the far path, which never grows the queue, until it overflows. I have not seen this outside batch mode.

Steps to reproduce (files attached)

  1. cp test/configs/stm32h723.config .config && make olddefconfig && make
  2. valgrind --error-limit=no python klippy/klippy.py repro2.cfg -i repro2.gcode -o out.serial -v -d out/klipper.dict -l klippy.log

repro2.cfg is a cartesian printer with an extruder, a synced [extruder_stepper gear] and a [gcode_macro CUBE] holding about 980 moves from a small cube; repro2.gcode is G28, M83, CUBE. The run exits 0, but valgrind reports:

Invalid write of size 4
   at 0x8F597A7: UnknownInlinedFun (stepcompress.c:441)
   by 0x8F597A7: queue_append (stepcompress.c:495)
   by 0x8F59C31: UnknownInlinedFun (stepcompress.c:526)
   by 0x8F59C31: itersolve_gen_steps_range (itersolve.c:108)
   ...
 Address 0x88d9770 is 0 bytes after a block of size 4,096 alloc'd
   at 0x4EDE250: realloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x8F598C0: UnknownInlinedFun (stepcompress.c:472)

stepcompress.c:441 is the store in queue_append_far() and :472 is the realloc() in queue_append_extend(). ERROR SUMMARY: 3 errors from 2 contexts.

Possible fix

Fall back to queue_append_extend() when the queue is full (valgrind reports 0 errors with this applied):

+static int queue_append_extend(struct stepcompress *sc);
+
 // Slow path for queue_append() - handle next step far in future
 static int
 queue_append_far(struct stepcompress *sc)
@@
     if (step_clock >= sc->last_step_clock + CLOCK_DIFF_MAX)
         return stepcompress_flush_far(sc, step_clock);
+    if (sc->queue_next >= sc->queue_end) {
+        // Queue full: grow/shuffle it instead of writing past queue_end
+        sc->next_step_clock = step_clock;
+        return queue_append_extend(sc);
+    }
     sc->queue_next->clock32 = step_clock;
     sc->queue_next++;
     return 0;

I am happy to submit this as a pull request.

Disclosure:

I used an AI coding assistant (Claude Code) to help investigate and draft this post. The attached klippy.log and valgrind output are unedited output from real runs on unmodified upstream.

klipper-queue_append_far-report.zip (11.6 KB)

Attached: klippy.log, repro2.cfg, repro2.gcode, valgrind output, patch.

Best Regards.
Guilherme Costa
Blocks Software Developer

2

Thanks for reporting. I created stepcompress: Fix possible out-of-bounds memory write by KevinOConnor · Pull Request #7421 · Klipper3d/klipper · GitHub which will hopefully fix the issue.

-Kevin

3

Thanks Kevin, that was quick.

I applied #7421 on top of 0cee841e3 and re-ran the attached repro2 case under valgrind: ERROR SUMMARY 0 errors (3 before the patch).
The overflow path is still hit once during the run, and it now goes through queue_append_extend() as expected.

Best Regards,
Guilherme Costa
Blocks Software Developer

1 Like