On 4/3/26 12:04 PM, Omar Polo wrote:
Renaud Allard <[email protected]> wrote:The p_start+diff_type stall check can be bypassed when intuit_diff_type alternates between detecting different types at different positions within the same garbage. Simpler fix: track p_base (the scan start position). If it hasn't advanced, stop. Fuzzed for 5 minutes with UBSan on amd64 and strict malloc on i386: 0 crashes. All 20 regression tests pass. Although I am not sure the 20 regression tests are sufficient Index: usr.bin/patch/patch.c =================================================================== RCS file: /cvs/src/usr.bin/patch/patch.c,v retrieving revision 1.78 diff -u -p -r1.78 patch.c --- usr.bin/patch/patch.c 23 Feb 2026 16:40:45 -0000 1.78 +++ usr.bin/patch/patch.c @@ -293,6 +293,12 @@ main(int argc, char *argv[]) /* for ed script just up and do it and exit */ if (diff_type == ED_DIFF) { do_ed_script(); + if (ofp) + fclose(ofp); + ofp = NULL; + if (rejfp) + fclose(rejfp); + rejfp = NULL; continue; }this part is okay op@, we need to clean up these files before next iteration.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)this part I'm less sure. if we're stuck here it means that we're detecting something but then the rest of the patch machinery fails to actually consume it, and we loop, am i right?
You are completely right.
then, why don't just advance one line, and re-enter intuit_diff_type? it would guarantee that we reach eof at some point and exit.
I have been going back and forth with your idea.A user would maybe want patch to continue. But intuit_diff_type picks up any line starting with "@@ -" as a unified diff and also extracts filenames from "---"/"+++" lines. Garbled input could match these patterns and cause patch to silently write to unintended files or create files with arbitrary names. It think stopping is safer even if frustrating.
smime.p7s
Description: S/MIME Cryptographic Signature
