git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Date
Jul 23, 2014, 11:43 UTC
Message-ID
<1406115795-24082-5-git-send-email-pclouds@gmail.com>
In-Reply-To
<1406115795-24082-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 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
Previous: Junio C HamanoNext: Nguyễn Thái Ngọc Duy
Message 20 of 36 in “nd/multiple-work-trees follow-ups”
  1. 0/5 nd/multiple-work-trees follow-upsNguyễn Thái Ngọc Duy, Jul 23, 2014
  2. 1/5 gitrepository-layout.txt: s/ignored/ignored if/Nguyễn Thái Ngọc Duy, Jul 23, 2014
  3. 2/5 prune --repos: fix uninitialized accessNguyễn Thái Ngọc Duy, Jul 23, 2014
  4. Junio C HamanoJul 23, 2014
  5. Duy NguyenJul 24, 2014
  6. 3/5 checkout --to: no auto-detach if the ref is already checked outNguyễn Thái Ngọc Duy, Jul 23, 2014
  7. Michael J GruberJul 23, 2014
  8. Junio C HamanoJul 23, 2014
  9. Duy NguyenJul 24, 2014
  10. Junio C HamanoJul 24, 2014
  11. Michael J GruberJul 25, 2014
  12. Junio C HamanoJul 30, 2014
  13. Junio C HamanoJul 30, 2014
  14. Duy NguyenAug 27, 2014
  15. Junio C HamanoAug 27, 2014
  16. Junio C HamanoJul 23, 2014
  17. Duy NguyenJul 24, 2014
  18. Junio C HamanoJul 24, 2014
  19. Junio C HamanoJul 24, 2014
  20. 4/5 checkout --to: fix dangling pointers in remove_junk()Nguyễn Thái Ngọc Duy, Jul 23, 2014
  21. 5/5 environment.c: fix incorrect git_graft_file initializationNguyễn Thái Ngọc Duy, Jul 23, 2014
  22. Junio C HamanoJul 23, 2014
  23. 0/8 nd/multiple-work-trees follow-upsNguyễn Thái Ngọc Duy, Jul 29, 2014
  24. 1/8 gitrepository-layout.txt: s/ignored/ignored if/Nguyễn Thái Ngọc Duy, Jul 29, 2014
  25. 2/8 checkout: no need to call check_linked_checkouts if head_ref is NULLNguyễn Thái Ngọc Duy, Jul 29, 2014
  26. 3/8 prune --repos: fix uninitialized accessNguyễn Thái Ngọc Duy, Jul 29, 2014
  27. 4/8 checkout: no auto-detach if the ref is already checked outNguyễn Thái Ngọc Duy, Jul 29, 2014
  28. 5/8 checkout --to: fix dangling pointers in remove_junk()Nguyễn Thái Ngọc Duy, Jul 29, 2014
  29. 6/8 environment.c: fix incorrect git_graft_file initializationNguyễn Thái Ngọc Duy, Jul 29, 2014
  30. 7/8 checkout: prefix --to argument properly when cwd is movedNguyễn Thái Ngọc Duy, Jul 29, 2014
  31. Junio C HamanoJul 29, 2014
  32. Duy NguyenJul 30, 2014
  33. 8/8 checkout --to: do not touch existing target directoryNguyễn Thái Ngọc Duy, Jul 29, 2014
  34. Junio C HamanoJul 30, 2014
  35. Duy NguyenJul 31, 2014
  36. Junio C HamanoJul 31, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.