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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 <pclouds@xxxxxxxxx>
---
 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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]