From 71c791605365d1873ef631bfc478fcd75080a063 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Justo?= Date: Tue, 23 Apr 2024 00:54:05 +0200 Subject: apply: plug a leak in apply_data MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We have an execution path in apply_data that leaks the local struct image. Plug it. This leak can be triggered with: $ echo foo >file $ git add file && git commit -m file $ echo bar >file $ git diff file >diff $ sed s/foo/frotz/ baddiff $ git apply --cached Signed-off-by: Junio C Hamano diff --git a/apply.c b/apply.c index 34f2032..2f752d7 100644 --- a/apply.c +++ b/apply.c @@ -3712,8 +3712,10 @@ static int apply_data(struct apply_state *state, struct patch *patch, fprintf(stderr, _("Falling back to direct application...\n")); /* Note: with --reject, apply_fragments() returns 0 */ - if (patch->direct_to_threeway || apply_fragments(state, &image, patch) < 0) + if (patch->direct_to_threeway || apply_fragments(state, &image, patch) < 0) { + clear_image(&image); return -1; + } } patch->result = image.buf; patch->resultsize = image.len; diff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh index c4f9bf0..c40b661 100755 --- a/t/t2016-checkout-patch.sh +++ b/t/t2016-checkout-patch.sh @@ -2,6 +2,7 @@ test_description='git checkout --patch' +TEST_PASSES_SANITIZE_LEAK=true . ./lib-patch-mode.sh test_expect_success 'setup' ' diff --git a/t/t4103-apply-binary.sh b/t/t4103-apply-binary.sh index d370ecf..144619a 100755 --- a/t/t4103-apply-binary.sh +++ b/t/t4103-apply-binary.sh @@ -9,6 +9,7 @@ test_description='git apply handling binary patches GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'setup' ' diff --git a/t/t4104-apply-boundary.sh b/t/t4104-apply-boundary.sh index 71ef413..dc501aa 100755 --- a/t/t4104-apply-boundary.sh +++ b/t/t4104-apply-boundary.sh @@ -5,6 +5,7 @@ test_description='git apply boundary tests' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh L="c d e f g h i j k l m n o p q r s t u v w x" diff --git a/t/t4113-apply-ending.sh b/t/t4113-apply-ending.sh index 66fa515..2c65c6a 100755 --- a/t/t4113-apply-ending.sh +++ b/t/t4113-apply-ending.sh @@ -6,6 +6,7 @@ test_description='git apply trying to add an ending line. ' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh # setup diff --git a/t/t4117-apply-reject.sh b/t/t4117-apply-reject.sh index c86d05a..4d15ccd 100755 --- a/t/t4117-apply-reject.sh +++ b/t/t4117-apply-reject.sh @@ -7,6 +7,7 @@ test_description='git apply with rejects ' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success setup ' diff --git a/t/t4123-apply-shrink.sh b/t/t4123-apply-shrink.sh index 3ef8461..3601c0c 100755 --- a/t/t4123-apply-shrink.sh +++ b/t/t4123-apply-shrink.sh @@ -2,6 +2,7 @@ test_description='apply a patch that is larger than the preimage' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh cat >F <<\EOF diff --git a/t/t4252-am-options.sh b/t/t4252-am-options.sh index e758e63..5b680dc 100755 --- a/t/t4252-am-options.sh +++ b/t/t4252-am-options.sh @@ -1,6 +1,8 @@ #!/bin/sh test_description='git am with options and not losing them' + +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh tm="$TEST_DIRECTORY/t4252" diff --git a/t/t4258-am-quoted-cr.sh b/t/t4258-am-quoted-cr.sh index 201915b..3573c91 100755 --- a/t/t4258-am-quoted-cr.sh +++ b/t/t4258-am-quoted-cr.sh @@ -2,6 +2,7 @@ test_description='test am --quoted-cr=' +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh DATA="$TEST_DIRECTORY/t4258" -- cgit v0.10.2-6-g49f6 From 5861aa84a7a5f393e953203068bd3fc5c710fc42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Justo?= Date: Tue, 23 Apr 2024 00:54:08 +0200 Subject: add-interactive: plug a leak in get_untracked_files MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plug a leak we have since ab1e1cccaf (built-in add -i: re-implement `add-untracked` in C, 2019-11-29). This leak can be triggered with: $ echo a | git add -i As a curiosity, we have a somewhat similar function in builtin/stash.c, which correctly frees the memory. Signed-off-by: Rubén Justo Signed-off-by: Junio C Hamano diff --git a/add-interactive.c b/add-interactive.c index 6bf87e7..e17602b 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -865,6 +865,7 @@ static int get_untracked_files(struct repository *r, } strbuf_release(&buf); + dir_clear(&dir); return 0; } -- cgit v0.10.2-6-g49f6 From ec9b74b18e019a8adff827ab22380f9771ac5f00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Justo?= Date: Tue, 23 Apr 2024 00:54:14 +0200 Subject: add-patch: plug a leak handling the '/' command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plug a leak we have since d6cf873340 (built-in add -p: implement the '/' ("search regex") command, 2019-12-13). This leak can be triggered with: $ printf "A\n\nB\n" >file $ git add file && git commit -m file $ printf "AA\n\nBB\n" >file $ printf "s\n/ .\n" >lines $ git add -p Signed-off-by: Junio C Hamano diff --git a/add-patch.c b/add-patch.c index a06dd18..0997d4a 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1646,6 +1646,7 @@ soft_increment: err(s, _("No hunk matches the given pattern")); break; } + regfree(®ex); hunk_index = i; } else if (s->answer.buf[0] == 's') { size_t splittable_into = hunk->splittable_into; -- cgit v0.10.2-6-g49f6 From 16727404c48ba1c2f43fb966276aee0b8cb24389 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Justo?= Date: Tue, 23 Apr 2024 00:54:18 +0200 Subject: add: plug a leak on interactive_add MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Plug a leak we have since 5a76aff1a6 (add: convert to use parse_pathspec, 2013-07-14). This leak can be triggered with: $ git add -p anything Fixing this leak allows us to mark as leak-free the following tests: + t3701-add-interactive.sh + t7514-commit-patch.sh Mark them with "TEST_PASSES_SANITIZE_LEAK=true" to notice and fix promply any new leak that may be introduced and triggered by them in the future. Signed-off-by: Rubén Justo Signed-off-by: Junio C Hamano diff --git a/builtin/add.c b/builtin/add.c index ae723bc..b7d3ff1 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -150,7 +150,7 @@ static int refresh(int verbose, const struct pathspec *pathspec) int interactive_add(const char **argv, const char *prefix, int patch) { struct pathspec pathspec; - int unused; + int unused, ret; if (!git_config_get_bool("add.interactive.usebuiltin", &unused)) warning(_("the add.interactive.useBuiltin setting has been removed!\n" @@ -163,9 +163,12 @@ int interactive_add(const char **argv, const char *prefix, int patch) prefix, argv); if (patch) - return !!run_add_p(the_repository, ADD_P_ADD, NULL, &pathspec); + ret = !!run_add_p(the_repository, ADD_P_ADD, NULL, &pathspec); else - return !!run_add_i(the_repository, &pathspec); + ret = !!run_add_i(the_repository, &pathspec); + + clear_pathspec(&pathspec); + return ret; } static int edit_patch(int argc, const char **argv, const char *prefix) diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index bc55255..04d8333 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -4,6 +4,7 @@ test_description='add -i basic tests' GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh . "$TEST_DIRECTORY"/lib-terminal.sh diff --git a/t/t7514-commit-patch.sh b/t/t7514-commit-patch.sh index b4de10a..03ba0c0 100755 --- a/t/t7514-commit-patch.sh +++ b/t/t7514-commit-patch.sh @@ -1,6 +1,8 @@ #!/bin/sh test_description='hunk edit with "commit -p -m"' + +TEST_PASSES_SANITIZE_LEAK=true . ./test-lib.sh test_expect_success 'setup (initial)' ' -- cgit v0.10.2-6-g49f6