Skip to content

Commit 4369e3a

Browse files
avargitster
authored andcommitted
hooks: fix "invoked hook" regression in a8cc594
Fix a regression in a8cc594 (hooks: fix an obscure TOCTOU "did we just run a hook?" race, 2022-03-07): The "invoked_hook" variable passed to run_commit_hook() wasn't passed forward to run_hooks_opt(), as push_to_checkout() in that commit correctly did. Whether we ran the code contingent on having run the hook or not was thus undefined, but in practice on most (all?) modern platforms we'd have run it (almost?) all the time, since stack variables will get initialized to some random value, which most of the time isn't "0". This bug was revealed by running e.g. "t5537-fetch-shallow.sh" with the --valgrind option. Unfortunately running the whole test suite with --valgrind is really slow, so we didn't have a CI job that spotted this. The --valgrind output was: ==31275== Conditional jump or move depends on uninitialised value(s) ==31275== at 0x43C63F: prepare_to_commit (commit.c:1058) ==31275== by 0x4396A5: cmd_commit (commit.c:1722) ==31275== by 0x407C8A: run_builtin (git.c:465) ==31275== by 0x406741: handle_builtin (git.c:718) ==31275== by 0x407665: run_argv (git.c:785) ==31275== by 0x406500: cmd_main (git.c:916) ==31275== by 0x510839: main (common-main.c:56) ==31275== Uninitialised value was created by a stack allocation ==31275== at 0x43B344: prepare_to_commit (commit.c:719) Reported-by: Jonathan Tan <[email protected]> Signed-off-by: Ævar Arnfjörð Bjarmason <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent a8cc594 commit 4369e3a

File tree

1 file changed

+1
-0
lines changed

1 file changed

+1
-0
lines changed

commit.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1732,5 +1732,6 @@ int run_commit_hook(int editor_is_used, const char *index_file,
17321732
strvec_push(&opt.args, arg);
17331733
va_end(args);
17341734

1735+
opt.invoked_hook = invoked_hook;
17351736
return run_hooks_opt(name, &opt);
17361737
}

0 commit comments

Comments
 (0)