On 4/3/26 2:00 PM, Kirill A. Korinsky wrote:
On Fri, 03 Apr 2026 12:51:01 +0200, Renaud Allard <[email protected]> wrote:[1 <text/plain; UTF-8 (base64)>] 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.So, you withdraw the second hunk from your diff and would like to keep only the first one, with fclose(), right?
You mean you would like the first one to be committed and discuss the second one separately? The second issue blocks patch, so it should also be solved. The question is what is the fully correct solution. I would prefer it to just stop instead of block or worse create lots of unintended files that are hard to remove.
smime.p7s
Description: S/MIME Cryptographic Signature
