On 05/04/2026 21:08, Kirill A. Korinsky wrote:
On Fri, 03 Apr 2026 14:46:33 +0200, Renaud Allard <[email protected]> wrote:This is the stall detection part from the earlier patch(1) report, split out for separate discussion. The fd leak fix (patch.c fclose) has ok op@. Problem: when intuit_diff_type() detects an ed script in malformed input, do_ed_script() may fail to advance the file position. On the next iteration, intuit_diff_type() finds the same pattern at the same position and the main loop repeats indefinitely. After hundreds of iterations, malloc detects heap corruption and aborts. Fix: track p_base across calls. If it hasn't advanced since the last call, the patch machinery failed to consume whatever was detected. Stop and treat the rest as garbage. The alternative of skipping one line and retrying was discussed. It guarantees reaching EOF but risks silent corruption: intuit_diff_type picks up any line starting with "@@ -" as a unified diff, and extracts filenames from "---"/"+++" lines. Garbled input could match these patterns, causing patch to write to unintended files or create files with arbitrary names. Stopping seems safer. The same bug is present in NetBSD, FreeBSD, and DragonFlyBSD (shared Larry Wall codebase). Index: usr.bin/patch/pch.c =================================================================== RCS file: /cvs/src/usr.bin/patch/pch.c,v retrieving revision 1.66 diff -u -p -r1.66 pch.c --- usr.bin/patch/pch.c 12 Jul 2023 15:45:34 -0000 1.66 +++ usr.bin/patch/pch.c @@ -180,6 +180,7 @@ static char *posix_name(const struct fil bool there_is_another_patch(void) { + static off_t prev_p_base = -1; bool exists = false; if (p_base != 0 && p_base >= p_filesize) { @@ -190,6 +191,12 @@ there_is_another_patch(void) if (verbose) say("Hmm..."); diff_type = intuit_diff_type(); + if (diff_type && p_base == prev_p_base) { + if (verbose) + say(" Ignoring the trailing garbage.\ndone\n"); + return false; + } + prev_p_base = p_base; if (!diff_type) { if (p_base != 0) { if (verbose)It has interesting side effect. another_hunk() exists with false when out_of_mem becames true, and old code just moved to the next hunk. After your diff it thinks that it is a loop and exists. Not sure how to reproduce it, nor that old behaviour is right one.
You are completely right there. Reproducing it in real life seems highly unlikely, but the issue is there. Right now, I don't see how to fix that without touching the main loop, but that would be a way bigger patch.
smime.p7s
Description: S/MIME Cryptographic Signature
