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. -- wbr, Kirill
