diff options
Diffstat (limited to 'dev-vcs/git/files')
| -rw-r--r-- | dev-vcs/git/files/git-2.47.0-maintenance.patch | 85 |
1 files changed, 85 insertions, 0 deletions
diff --git a/dev-vcs/git/files/git-2.47.0-maintenance.patch b/dev-vcs/git/files/git-2.47.0-maintenance.patch new file mode 100644 index 000000000000..085d4adac33f --- /dev/null +++ b/dev-vcs/git/files/git-2.47.0-maintenance.patch @@ -0,0 +1,85 @@ +https://lore.kernel.org/git/CAG=Um+0mJW-oAH+YLC3dWEU64JwS-zMkkTiFWYBe4g6HMbe-iA@mail.gmail.com/ +https://github.com/git/git/commit/c95547a394a35dc26afa686454086d2db6e51ea4 + +From c95547a394a35dc26afa686454086d2db6e51ea4 Mon Sep 17 00:00:00 2001 +From: Patrick Steinhardt <ps@pks.im> +Date: Thu, 10 Oct 2024 07:33:01 +0200 +Subject: [PATCH] builtin/gc: fix crash when running `git maintenance start` + +It was reported on the mailing list that running `git maintenance start` +immediately segfaults starting with b6c3f8e12c (builtin/maintenance: fix +leak in `get_schedule_cmd()`, 2024-09-26). And indeed, this segfault is +trivial to reproduce up to a point where one is scratching their head +why we didn't catch this regression in our test suite. + +The root cause of this error is `get_schedule_cmd()`, which does not +populate the `out` parameter in all cases anymore starting with the +mentioned commit. Callers do assume it to always be populated though and +will e.g. call `strvec_split()` on the returned value, which will of +course segfault when the variable is uninitialized. + +So why didn't we catch this trivial regression? The reason is that our +tests always set up the "GIT_TEST_MAINT_SCHEDULER" environment variable +via "t/test-lib.sh", which allows us to override the scheduler command +with a custom one so that we don't accidentally modify the developer's +system. But the faulty code where we don't set the `out` parameter will +only get hit in case that environment variable is _not_ set, which is +never the case when executing our tests. + +Fix the regression by again unconditionally allocating the value in the +`out` parameter, if provided. Add a test that unsets the environment +variable to catch future regressions in this area. + +Reported-by: Shubham Kanodia <shubham.kanodia10@gmail.com> +Signed-off-by: Patrick Steinhardt <ps@pks.im> +Signed-off-by: Junio C Hamano <gitster@pobox.com> +--- a/builtin/gc.c ++++ b/builtin/gc.c +@@ -1794,7 +1794,7 @@ static const char *get_frequency(enum schedule_priority schedule) + * | Input | Output | + * | *cmd | return code | *out | *is_available | + * +-------+-------------+-------------------+---------------+ +- * | "foo" | false | NULL | (unchanged) | ++ * | "foo" | false | "foo" (allocated) | (unchanged) | + * +-------+-------------+-------------------+---------------+ + * + * GIT_TEST_MAINT_SCHEDULER set to “foo:./mock_foo.sh,bar:./mock_bar.sh” +@@ -1812,8 +1812,11 @@ static int get_schedule_cmd(const char *cmd, int *is_available, char **out) + struct string_list_item *item; + struct string_list list = STRING_LIST_INIT_NODUP; + +- if (!testing) ++ if (!testing) { ++ if (out) ++ *out = xstrdup(cmd); + return 0; ++ } + + if (is_available) + *is_available = 0; +--- a/t/t7900-maintenance.sh ++++ b/t/t7900-maintenance.sh +@@ -646,6 +646,22 @@ test_expect_success !MINGW 'register and unregister with regex metacharacters' ' + maintenance.repo "$(pwd)/$META" + ' + ++test_expect_success 'start without GIT_TEST_MAINT_SCHEDULER' ' ++ test_when_finished "rm -rf systemctl.log script repo" && ++ mkdir script && ++ write_script script/systemctl <<-\EOF && ++ echo "$*" >>../systemctl.log ++ EOF ++ git init repo && ++ ( ++ cd repo && ++ sane_unset GIT_TEST_MAINT_SCHEDULER && ++ PATH="$PWD/../script:$PATH" git maintenance start --scheduler=systemd ++ ) && ++ test_grep -- "--user list-timers" systemctl.log && ++ test_grep -- "enable --now git-maintenance@" systemctl.log ++' ++ + test_expect_success 'start --scheduler=<scheduler>' ' + test_expect_code 129 git maintenance start --scheduler=foo 2>err && + test_grep "unrecognized --scheduler argument" err && + |