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