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

Re: [PATCH v2] reflog: close leak of reflog expire entry

From
Lidong Yan <yldhome2d2@gmail.com>
Date
Jul 10, 2025, 03:00 UTC
Message-ID
<D34FE2DE-EE5B-43F3-A706-1AC133AA72F1@gmail.com>
In-Reply-To
<20250709-jk-fix-leak-reflog-expire-config-v2-1-f9af934be8c1@gmail.com>
Jacob Keller <jacob.e.keller@intel.com> wrote:
Show 91 quoted lines
> 
> From: Jacob Keller <jacob.keller@gmail.com>
> 
> find_cfg_ent() allocates a struct reflog_expire_entry_option via
> FLEX_ALLOC_MEM and inserts it into a linked list in the
> reflog_expire_options structure. The entries in this list are never
> freed, resulting in a leak in cmd_reflog_expire and the gc reflog expire
> maintenance task:
> 
> Direct leak of 39 byte(s) in 1 object(s) allocated from:
>    #0 0x7ff975ee6883 in calloc (/lib64/libasan.so.8+0xe6883)
>    #1 0x0000010edada in xcalloc ../wrapper.c:154
>    #2 0x000000df0898 in find_cfg_ent ../reflog.c:28
>    #3 0x000000df0898 in reflog_expire_config ../reflog.c:70
>    #4 0x00000095c451 in configset_iter ../config.c:2116
>    #5 0x0000006d29e7 in git_config ../config.h:724
>    #6 0x0000006d29e7 in cmd_reflog_expire ../builtin/reflog.c:205
>    #7 0x0000006d504c in cmd_reflog ../builtin/reflog.c:419
>    #8 0x0000007e4054 in run_builtin ../git.c:480
>    #9 0x0000007e4054 in handle_builtin ../git.c:746
>    #10 0x0000007e8a35 in run_argv ../git.c:813
>    #11 0x0000007e8a35 in cmd_main ../git.c:953
>    #12 0x000000441e8f in main ../common-main.c:9
>    #13 0x7ff9754115f4 in __libc_start_call_main (/lib64/libc.so.6+0x35f4)
>    #14 0x7ff9754116a7 in __libc_start_main@@GLIBC_2.34 (/lib64/libc.so.6+0x36a7)
>    #15 0x000000444184 in _start (/home/jekeller/libexec/git-core/git+0x444184)
> 
> Close this leak by adding a reflog_clear_expire_config() function which
> iterates the linked list and frees its elements. Call it upon exit of
> cmd_reflog_expire() and in reflog_expiry_cleanup().
> 
> Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
> ---
> Changes in v2:
> - Actually fix the leak properly. (Thanks Jeff for catching my brain fart!)
> - Link to v1: https://lore.kernel.org/r/20250709-jk-fix-leak-reflog-expire-config-v1-1-34d5461cf8f5@gmail.com
> ---
> reflog.h         |  2 ++
> builtin/reflog.c |  3 +++
> reflog.c         | 15 +++++++++++++++
> 3 files changed, 20 insertions(+)
> 
> diff --git a/reflog.h b/reflog.h
> index 63bb56280f4e..74b3f3c4f0ac 100644
> --- a/reflog.h
> +++ b/reflog.h
> @@ -34,6 +34,8 @@ struct reflog_expire_options {
> int reflog_expire_config(const char *var, const char *value,
> const struct config_context *ctx, void *cb);
> 
> +void reflog_clear_expire_config(struct reflog_expire_options *opts);
> +
> /*
>  * Adapt the options so that they apply to the given refname. This applies any
>  * per-reference reflog expiry configuration that may exist to the options.
> diff --git a/builtin/reflog.c b/builtin/reflog.c
> index 3acaf3e32c27..d4da41aaea73 100644
> --- a/builtin/reflog.c
> +++ b/builtin/reflog.c
> @@ -283,6 +283,9 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix,
>     &cb);
> free(ref);
> }
> +
> + reflog_clear_expire_config(&opts);
> +
> return status;
> }
> 
> diff --git a/reflog.c b/reflog.c
> index 15d81ebea978..3ce1780924dd 100644
> --- a/reflog.c
> +++ b/reflog.c
> @@ -81,6 +81,20 @@ int reflog_expire_config(const char *var, const char *value,
> return 0;
> }
> 
> +void reflog_clear_expire_config(struct reflog_expire_options *opts)
> +{
> + struct reflog_expire_entry_option *ent = opts->entries, *tmp;
> +
> + while (ent) {
> + tmp = ent;
> + ent = ent->next;
> + free(tmp);
> + }
> +
> + opts->entries = NULL;
> + opts->entries_tail = NULL;
> +}
> +
This looks correct.
Show 12 quoted lines
> void reflog_expire_options_set_refname(struct reflog_expire_options *cb,
>       const char *ref)
> {
> @@ -490,6 +504,7 @@ void reflog_expiry_cleanup(void *cb_data)
> for (elem = cb->mark_list; elem; elem = elem->next)
> clear_commit_marks(elem->item, REACHABLE);
> free_commit_list(cb->mark_list);
> + reflog_clear_expire_config(&cb->opts);
> }
> 
> int count_reflog_ent(struct object_id *ooid UNUSED,
> 
In builtin/reflog.c, we have code like
---
	for (i = 0; i < argc; i++) {
		char *ref;
		struct expire_reflog_policy_cb cb = { .opts = opts };
		if (!repo_dwim_log(the_repository, argv[i], strlen(argv[i]), NULL, &ref)) {
			status |= error(_("reflog could not be found: '%s'"), argv[i]);
			continue;
		}
		reflog_expire_options_set_refname(&cb.opts, ref);
		status |= refs_reflog_expire(get_main_ref_store(the_repository),
					     ref, flags,
					     reflog_expiry_prepare,
					     should_prune_fn,
					     reflog_expiry_cleanup,
					     &cb);
		free(ref);
	}
+      reflog_clear_expire_config(&opts);
---

I think allowing reblog_expiry_cleanup() to free all opt->entries might cause reblog_expire_options_set_refname() to behave incorrectly.

Previous: Jacob KellerNext: Jeff King
Message 2 of 4 in “reflog: close leak of reflog expire entry”
  1. reflog: close leak of reflog expire entryJacob Keller, Jul 9, 2025
  2. Lidong YanJul 10, 2025
  3. Jeff KingJul 10, 2025
  4. Jacob KellerJul 10, 2025

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.