[PATCH 4/5] checkout --to: fix dangling pointers in remove_junk()

[Nguyễn Thái Ngọc Duy]

junk_git_dir is set to sb_repo.buf. By the end of prepare_linked_checkout(),
sb_repo is freed and so junk_git_dir points to nowhere. If the second
checkout command fails, is_junk remains non-zero, remove_junk() will
be called and try to clean junk_git_dir, which could be anything now
(if it does not crash the program).

The new test may pass even without this patch. But it does fail under
valgrind (without this patch) with "Invalid read of size 8" at the
right line.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclo...@gmail.com>
---
 builtin/checkout.c     | 15 ++++++++++-----
 t/t2025-checkout-to.sh |  6 ++++++
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/builtin/checkout.c b/builtin/checkout.c
index d35245a..e62c084 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -825,8 +825,8 @@ static int switch_branches(const struct checkout_opts *opts,
        return ret || writeout_error;
 }
 
-static const char *junk_work_tree;
-static const char *junk_git_dir;
+static char *junk_work_tree;
+static char *junk_git_dir;
 static int is_junk;
 static pid_t junk_pid;
 
@@ -895,7 +895,7 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
 
        if (mkdir(sb_repo.buf, 0777))
                die_errno(_("could not create directory of '%s'"), sb_repo.buf);
-       junk_git_dir = sb_repo.buf;
+       junk_git_dir = xstrdup(sb_repo.buf);
        is_junk = 1;
 
        /*
@@ -909,7 +909,7 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
        if (safe_create_leading_directories_const(sb_git.buf))
                die_errno(_("could not create leading directories of '%s'"),
                          sb_git.buf);
-       junk_work_tree = path;
+       junk_work_tree = xstrdup(path);
 
        strbuf_reset(&sb);
        strbuf_addf(&sb, "%s/gitdir", sb_repo.buf);
@@ -939,8 +939,13 @@ static int prepare_linked_checkout(const struct 
checkout_opts *opts,
        cp.git_cmd = 1;
        cp.argv = opts->saved_argv;
        ret = run_command(&cp);
-       if (!ret)
+       if (!ret) {
                is_junk = 0;
+               free(junk_work_tree);
+               free(junk_git_dir);
+               junk_work_tree = NULL;
+               junk_git_dir = NULL;
+       }
        strbuf_reset(&sb);
        strbuf_addf(&sb, "%s/locked", sb_repo.buf);
        unlink_or_warn(sb.buf);
diff --git a/t/t2025-checkout-to.sh b/t/t2025-checkout-to.sh
index c6601a4..8a00310 100755
--- a/t/t2025-checkout-to.sh
+++ b/t/t2025-checkout-to.sh
@@ -12,6 +12,12 @@ test_expect_success 'checkout --to not updating paths' '
        test_must_fail git checkout --to -- init.t
 '
 
+test_expect_success 'checkout --to refuses to checkout locked branch' '
+       test_must_fail git checkout --to zere master &&
+       ! test -d zere &&
+       ! test -d .git/repos/zere
+'
+
 test_expect_success 'checkout --to a new worktree' '
        git rev-parse HEAD >expect &&
        git checkout --detach --to here master &&
-- 
1.9.1.346.ga2b5940

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html