git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Junio C Hamano" <gitster@pobox.com>,
	"Max Kirillov" <max@max630.net>,
	"Eric Sunshine" <sunshine@sunshineco.com>,
	git@drmicha.warpmail.net,
	"Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH v2 5/8] checkout --to: fix dangling pointers in remove_junk()
Date: Tue, 29 Jul 2014 20:50:28 +0700	[thread overview]
Message-ID: <1406641831-2390-6-git-send-email-pclouds@gmail.com> (raw)
In-Reply-To: <1406641831-2390-1-git-send-email-pclouds@gmail.com>

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@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 0714856..173aab1 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -820,8 +820,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;
 
@@ -890,7 +890,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;
 
 	/*
@@ -904,7 +904,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);
@@ -934,8 +934,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 &&
-- 
2.1.0.rc0.78.gc0d8480

  parent reply	other threads:[~2014-07-29 13:53 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-23 11:43 [PATCH 0/5] nd/multiple-work-trees follow-ups Nguyễn Thái Ngọc Duy
2014-07-23 11:43 ` [PATCH 1/5] gitrepository-layout.txt: s/ignored/ignored if/ Nguyễn Thái Ngọc Duy
2014-07-23 11:43 ` [PATCH 2/5] prune --repos: fix uninitialized access Nguyễn Thái Ngọc Duy
2014-07-23 19:59   ` Junio C Hamano
2014-07-24 10:14     ` Duy Nguyen
2014-07-23 11:43 ` [PATCH 3/5] checkout --to: no auto-detach if the ref is already checked out Nguyễn Thái Ngọc Duy
2014-07-23 13:48   ` Michael J Gruber
2014-07-23 17:46     ` Junio C Hamano
2014-07-24  9:58     ` Duy Nguyen
2014-07-24 21:30       ` Junio C Hamano
2014-07-25  6:51         ` Michael J Gruber
2014-07-30 18:03           ` Junio C Hamano
2014-07-30 18:52             ` Junio C Hamano
2014-08-27 11:58             ` Duy Nguyen
2014-08-27 16:08               ` Junio C Hamano
2014-07-23 21:16   ` Junio C Hamano
2014-07-24 10:09     ` Duy Nguyen
2014-07-24 16:39       ` Junio C Hamano
2014-07-24 18:13         ` Junio C Hamano
2014-07-23 11:43 ` [PATCH 4/5] checkout --to: fix dangling pointers in remove_junk() Nguyễn Thái Ngọc Duy
2014-07-23 11:43 ` [PATCH 5/5] environment.c: fix incorrect git_graft_file initialization Nguyễn Thái Ngọc Duy
2014-07-23 21:22   ` Junio C Hamano
2014-07-29 13:50 ` [PATCH v2 0/8] nd/multiple-work-trees follow-ups Nguyễn Thái Ngọc Duy
2014-07-29 13:50   ` [PATCH v2 1/8] gitrepository-layout.txt: s/ignored/ignored if/ Nguyễn Thái Ngọc Duy
2014-07-29 13:50   ` [PATCH v2 2/8] checkout: no need to call check_linked_checkouts if head_ref is NULL Nguyễn Thái Ngọc Duy
2014-07-29 13:50   ` [PATCH v2 3/8] prune --repos: fix uninitialized access Nguyễn Thái Ngọc Duy
2014-07-29 13:50   ` [PATCH v2 4/8] checkout: no auto-detach if the ref is already checked out Nguyễn Thái Ngọc Duy
2014-07-29 13:50   ` Nguyễn Thái Ngọc Duy [this message]
2014-07-29 13:50   ` [PATCH v2 6/8] environment.c: fix incorrect git_graft_file initialization Nguyễn Thái Ngọc Duy
2014-07-29 13:50   ` [PATCH v2 7/8] checkout: prefix --to argument properly when cwd is moved Nguyễn Thái Ngọc Duy
2014-07-29 20:51     ` Junio C Hamano
2014-07-30 10:32       ` Duy Nguyen
2014-07-29 13:50   ` [PATCH v2 8/8] checkout --to: do not touch existing target directory Nguyễn Thái Ngọc Duy
2014-07-30 17:51 ` [PATCH 0/5] nd/multiple-work-trees follow-ups Junio C Hamano
2014-07-31 10:13   ` Duy Nguyen
2014-07-31 17:00     ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1406641831-2390-6-git-send-email-pclouds@gmail.com \
    --to=pclouds@gmail.com \
    --cc=git@drmicha.warpmail.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=max@max630.net \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).