]> git.ipfire.org Git - thirdparty/git.git/commit - commit.c
hooks: fix "invoked hook" regression in a8cc5943338
authorÆvar Arnfjörð Bjarmason <avarab@gmail.com>
Mon, 21 Mar 2022 23:15:13 +0000 (00:15 +0100)
committerJunio C Hamano <gitster@pobox.com>
Wed, 23 Mar 2022 20:03:43 +0000 (13:03 -0700)
commit4369e3a1a39895ab51c2bef2985255ad05957a20
treefbaae1fd159728846de7cd4291cef93ec49fbc1d
parenta8cc594333848713b8e772cccf8159196ea85ede
hooks: fix "invoked hook" regression in a8cc5943338

Fix a regression in a8cc5943338 (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 <jonathantanmy@google.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
commit.c