"Alexandr Miloslavskiy via GitGitGadget" <gitgitgadget@xxxxxxxxx>
writes:
> From: Alexandr Miloslavskiy <alexandr.miloslavskiy@xxxxxxxxxxx>
>
> `real_path()` returns result from a shared buffer, inviting subtle
> reentrance bugs. One of these bugs occur when invoked this way:
> set_git_dir(real_path(git_dir))
>
> In this case, `real_path()` has reentrance:
> real_path
> read_gitfile_gently
> repo_set_gitdir
> setup_git_env
> set_git_dir_1
> set_git_dir
>
> Later, `set_git_dir()` uses its now-dead parameter:
> !is_absolute_path(path)
>
> Fix this by using a dedicated `strbuf` to hold `strbuf_realpath()`.
With this detailed explanation, I expected to see a test or two that
demonstrates a breakage, but reading a stale value may not
reproducibly give the same wrong result or crash the program,
perhaps?
> -void set_git_dir(const char *path)
> +void set_git_dir(const char *path, int make_realpath)
> {
> + struct strbuf realpath = STRBUF_INIT;
> +
> + if (make_realpath) {
> + strbuf_realpath(&realpath, path, 1);
> + path = realpath.buf;
> + }
> +
> set_git_dir_1(path);
> if (!is_absolute_path(path))
> chdir_notify_register(NULL, update_relative_gitdir, NULL);
> +
> + strbuf_release(&realpath);
> }
Makes sense. I looked at changes to the callers in this patch and
it all made sense.
