From: "Rubén Justo" <rjusto@gmail.com>
To: Git List <git@vger.kernel.org>
Subject: [PATCH 03/11] remote: fix a leak in query_matches_negative_refspec
Date: Sun, 11 Jun 2023 20:49:35 +0200 [thread overview]
Message-ID: <a9b27053-ff2b-7b67-f2ba-5691f4bda961@gmail.com> (raw)
In-Reply-To: <9eb931e7-dd18-8ce8-4d4a-cf3d85066189@gmail.com>
In c0192df630 (refspec: add support for negative refspecs, 2020-09-30)
query_matches_negative_refspec() was introduced.
The function was implemented as a two-loop process, where the former
loop accumulates and the latter evaluates. To accumulate, a string_list
is used.
Within the first loop, there are three cases where a string is added to
the string_list. Two of them add strings that do not need to be
freed. But in the third case, the string added is returned by
match_name_with_pattern(), which needs to be freed.
The string_list is initialized with STRING_LIST_INIT_NODUP, i.e. when
cleared, the strings added are not freed. Therefore, the string
returned by match_name_with_pattern() is not freed, so we have a leak.
$ git remote add local .
$ git update-ref refs/remotes/local/foo HEAD
$ git branch --track bar local/foo
Direct leak of 24 byte(s) in 1 object(s) allocated from:
... in xrealloc wrapper.c
... in strbuf_grow strbuf.c
... in strbuf_add strbuf.c
... in match_name_with_pattern remote.c
... in query_matches_negative_refspec remote.c
... in query_refspecs remote.c
... in remote_find_tracking remote.c
... in find_tracked_branch branch.c
... in for_each_remote remote.c
... in setup_tracking branch.c
... in create_branch branch.c
... in cmd_branch builtin/branch.c
... in run_builtin git.c
Direct leak of 24 byte(s) in 1 object(s) allocated from:
... in xrealloc wrapper.c
... in strbuf_grow strbuf.c
... in strbuf_add strbuf.c
... in match_name_with_pattern remote.c
... in query_matches_negative_refspec remote.c
... in query_refspecs remote.c
... in remote_find_tracking remote.c
... in check_tracking_branch branch.c
... in for_each_remote remote.c
... in validate_remote_tracking_branch branch.c
... in dwim_branch_start branch.c
... in create_branch branch.c
... in cmd_branch builtin/branch.c
... in run_builtin git.c
An interesting point to note is that while string_list_append() is used
in the first two cases described, string_list_append_nodup() is used in
the third. This seems to indicate an intention to delegate the
responsibility for freeing the string, to the string_list. As if the
string_list had been initialized with STRING_LIST_INIT_DUP, i.e. the
strings are strdup()'d when added (except if the "_nodup" API is used)
and freed when cleared.
Switching to STRING_LIST_INIT_DUP fixes the leak and probably is what we
wanted to do originally. Let's do it.
Signed-off-by: Rubén Justo <rjusto@gmail.com>
---
remote.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/remote.c b/remote.c
index 0764fca0db..1bcd36e358 100644
--- a/remote.c
+++ b/remote.c
@@ -890,7 +890,7 @@ static int query_matches_negative_refspec(struct refspec *rs, struct refspec_ite
{
int i, matched_negative = 0;
int find_src = !query->src;
- struct string_list reversed = STRING_LIST_INIT_NODUP;
+ struct string_list reversed = STRING_LIST_INIT_DUP;
const char *needle = find_src ? query->dst : query->src;
/*
--
2.40.1
next prev parent reply other threads:[~2023-06-11 18:49 UTC|newest]
Thread overview: 47+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-11 18:29 tests: mark as passing with SANITIZE=leak Rubén Justo
2023-06-11 18:49 ` [PATCH 01/11] rev-parse: fix a leak with --abbrev-ref Rubén Justo
2023-06-12 3:12 ` Jeff King
2023-06-16 22:34 ` Rubén Justo
2023-06-11 18:49 ` [PATCH 02/11] config: fix a leak in git_config_copy_or_rename_section_in_file Rubén Justo
2023-06-12 3:14 ` Jeff King
2023-06-11 18:49 ` Rubén Justo [this message]
2023-06-12 3:17 ` [PATCH 03/11] remote: fix a leak in query_matches_negative_refspec Jeff King
2023-06-16 22:37 ` Rubén Justo
2023-06-11 18:49 ` [PATCH 04/11] branch: fix a leak in dwim_and_setup_tracking Rubén Justo
2023-06-12 3:21 ` Jeff King
2023-06-16 22:45 ` Rubén Justo
2023-06-11 18:49 ` [PATCH 05/11] branch: fix a leak in setup_tracking Rubén Justo
2023-06-12 3:26 ` Jeff King
2023-06-16 22:46 ` Rubén Justo
2023-06-11 18:50 ` [PATCH 06/11] branch: fix a leak in cmd_branch Rubén Justo
2023-06-12 3:46 ` Jeff King
2023-06-16 22:50 ` Rubén Justo
2023-06-11 18:50 ` [PATCH 07/11] branch: fix a leak in inherit_tracking Rubén Justo
2023-06-12 3:48 ` Jeff King
2023-06-11 18:50 ` [PATCH 08/11] branch: fix a leak in check_tracking_branch Rubén Justo
2023-06-12 3:55 ` Jeff King
2023-06-11 18:50 ` [PATCH 09/11] branch: fix a leak in setup_tracking Rubén Justo
2023-06-12 3:59 ` Jeff King
2023-06-11 18:50 ` [PATCH 10/11] config: fix a leak in git_config_copy_or_rename_section_in_file Rubén Justo
2023-06-12 4:05 ` Jeff King
2023-06-16 23:04 ` Rubén Justo
2023-06-11 18:50 ` [PATCH 11/11] tests: mark as passing with SANITIZE=leak Rubén Justo
2023-06-11 21:23 ` Rubén Justo
2023-06-12 4:06 ` Jeff King
2023-06-16 23:14 ` Rubén Justo
2023-06-13 19:34 ` Junio C Hamano
2023-06-16 23:27 ` [PATCH v2 0/5] " Rubén Justo
2023-06-16 23:34 ` [PATCH v2 1/5] rev-parse: fix a leak with --abbrev-ref Rubén Justo
2023-06-16 23:34 ` [PATCH v2 2/5] branch: fix a leak in setup_tracking Rubén Justo
2023-06-16 23:34 ` [PATCH v2 3/5] branch: fix a leak in cmd_branch Rubén Justo
2023-06-16 23:34 ` [PATCH v2 4/5] config: fix a leak in git_config_copy_or_rename_section_in_file Rubén Justo
2023-06-16 23:35 ` [PATCH v2 5/5] tests: mark as passing with SANITIZE=leak Rubén Justo
2023-06-16 23:45 ` [PATCH v2 0/5] " Junio C Hamano
2023-06-17 5:48 ` Jeff King
2023-06-17 6:35 ` Rubén Justo
2023-06-17 6:37 ` [PATCH v3 " Rubén Justo
2023-06-17 6:40 ` [PATCH v3 1/5] rev-parse: fix a leak with --abbrev-ref Rubén Justo
2023-06-17 6:41 ` [PATCH v3 2/5] branch: fix a leak in setup_tracking Rubén Justo
2023-06-17 6:41 ` [PATCH v3 3/5] branch: fix a leak in cmd_branch Rubén Justo
2023-06-17 6:41 ` [PATCH v3 4/5] config: fix a leak in git_config_copy_or_rename_section_in_file Rubén Justo
2023-06-17 6:41 ` [PATCH v3 5/5] tests: mark as passing with SANITIZE=leak Rubén Justo
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=a9b27053-ff2b-7b67-f2ba-5691f4bda961@gmail.com \
--to=rjusto@gmail.com \
--cc=git@vger.kernel.org \
/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).