Skip to content

Commit

Permalink
builtin/rebase: fix options.strategy memory lifecycle
Browse files Browse the repository at this point in the history
- cmd_rebase populates rebase_options.strategy with newly allocated
  strings, hence we need to free those strings at the end of cmd_rebase
  to avoid a leak.
- In some cases: get_replay_opts() is called, which prepares replay_opts
  using data from rebase_options. We used to simply copy the pointer,
  however that would now result in a double-free because
  sequencer_remove_state() is eventually used to free
  replay_opts.strategy. To avoid this we xstrdup() strategy when
  adding it to replay_opts. (The original leak happens because we always
  populate rebase_options.strategy, but we don't always enter the path
  that calls get_replay_opts() and later sequencer_remove_state() - in
  other words we'd sometimes but not always free rebase_options.strategy.
  Relying on such indirect free'ing would be impossible to understand
  anyhow.)

This was first seen when running t0021 with LSAN, but t2012 helped catch
the fact that we can't just free(options.strategy) at the end of
cmd_rebase (as that can cause a double-free). LSAN output from t0021:

LSAN output from t0021:

Direct leak of 4 byte(s) in 1 object(s) allocated from:
    #0 0x486804 in strdup ../projects/compiler-rt/lib/asan/asan_interceptors.cpp:452:3
    #1 0xa71eb8 in xstrdup wrapper.c:29:14
    #2 0x61b1cc in cmd_rebase builtin/rebase.c:1779:22
    #3 0x4ce83e in run_builtin git.c:475:11
    #4 0x4ccafe in handle_builtin git.c:729:3
    #5 0x4cb01c in run_argv git.c:818:4
    #6 0x4cb01c in cmd_main git.c:949:19
    #7 0x6b3fad in main common-main.c:52:11
    git#8 0x7f267b512349 in __libc_start_main (/lib64/libc.so.6+0x24349)

SUMMARY: AddressSanitizer: 4 byte(s) leaked in 1 allocation(s).

Signed-off-by: Andrzej Hunt <andrzej@ahunt.org>
  • Loading branch information
ahunt committed Jul 25, 2021
1 parent f90769b commit 3c09d80
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions builtin/rebase.c
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts)
replay.ignore_date = opts->ignore_date;
replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt);
if (opts->strategy)
replay.strategy = opts->strategy;
replay.strategy = xstrdup_or_null(opts->strategy);
else if (!replay.strategy && replay.default_strategy) {
replay.strategy = replay.default_strategy;
replay.default_strategy = NULL;
Expand Down Expand Up @@ -1713,7 +1713,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
int i;

if (!options.strategy)
options.strategy = "recursive";
options.strategy = xstrdup("recursive");

strbuf_reset(&buf);
for (i = 0; i < strategy_options.nr; i++)
Expand Down Expand Up @@ -2109,6 +2109,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
free(options.head_name);
free(options.gpg_sign_opt);
free(options.cmd);
free(options.strategy);
strbuf_release(&options.git_format_patch_opt);
free(squash_onto_name);
return ret;
Expand Down

0 comments on commit 3c09d80

Please sign in to comment.