threads / patch / 63773

patchreflog: close leak of reflog expire entry

Subject: [PATCH] reflog: close leak of reflog expire entry

## tl;dr

3 messages between Jul 9, 2025 and Jul 9, 2025. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Jacob Keller· Jul 9, 2025, 21:49 UTC · lore
From: Jacob Keller <jacob.keller@gmail.com>

find_cfg_ent() allocates a struct reflog_expire_entry_option via FLEX_ALLOC_MEM and returns its pointer to reflog_expire_config(). The function exits without freeing the memory:

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 freeing the entry pointer on exit of the reflog_expire_config() function. This frees both the entry structure and its embedded pattern array thanks to the use of FLEX_ALLOC_MEM.

Signed-off-by: Jacob Keller <jacob.keller@gmail.com>
---
 reflog.c | 3 +++
 1 file changed, 3 insertions(+)
Show changes to reflog.c +3 −1
diff --git a/reflog.c b/reflog.c
index 15d81ebea978..43647eaf89eb 100644
--- a/reflog.c
+++ b/reflog.c
@@ -78,6 +78,9 @@ int reflog_expire_config(const char *var, const char *value,
 		ent->expire_unreachable = expire;
 		break;
 	}
+
+	free(ent);
+
 	return 0;
 }
 

---
base-commit: a30f80fde927d70950b3b4d1820813480968fb0d
change-id: 20250709-jk-fix-leak-reflog-expire-config-712ca6dc685a

Best regards,
--  
Jacob Keller <jacob.keller@gmail.com>
Jeff King· Jul 9, 2025, 22:36 UTC · re: Jacob Keller · lore

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

On Wed, Jul 09, 2025 at 02:49:14PM -0700, Jacob Keller wrote:
Show 25 quoted lines
> find_cfg_ent() allocates a struct reflog_expire_entry_option via
> FLEX_ALLOC_MEM and returns its pointer to reflog_expire_config(). The
> function exits without freeing the memory:
> 
> 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 freeing the entry pointer on exit of the
> reflog_expire_config() function. This frees both the entry structure and
> its embedded pattern array thanks to the use of FLEX_ALLOC_MEM.

Hmm, this can't be right, can it? The end of reflog_expire_config() looks like this:

        ent = find_cfg_ent(opts, pattern, pattern_len);
        if (!ent)
                return -1;
        switch (slot) {
        case REFLOG_EXPIRE_TOTAL:
                ent->expire_total = expire;
                break;
        case REFLOG_EXPIRE_UNREACH:
                ent->expire_unreachable = expire;
                break;
        }
        return 0;

So if we free(ent), then what was the point of the function? We'd set some fields in it and then throw it away?

And indeed, find_cfg_ent() seems to add the newly allocated entry to the list opt->entries list. So by freeing here, we're leaving a dangling pointer in that list.

Probably that list needs to be cleaned up when cmd_reflog_expire() finishes?

-Peff
Keller, Jacob E· Jul 9, 2025, 23:24 UTC · re: Jeff King · lore

RE: [PATCH] reflog: close leak of reflog expire entry

Show 65 quoted lines
> -----Original Message-----
> From: Jeff King <peff@peff.net>
> Sent: Wednesday, July 9, 2025 3:37 PM
> To: Keller, Jacob E <jacob.e.keller@intel.com>
> Cc: git@vger.kernel.org; Junio C Hamano <gitster@pobox.com>; Jacob Keller
> <jacob.keller@gmail.com>
> Subject: Re: [PATCH] reflog: close leak of reflog expire entry
> 
> On Wed, Jul 09, 2025 at 02:49:14PM -0700, Jacob Keller wrote:
> 
> > find_cfg_ent() allocates a struct reflog_expire_entry_option via
> > FLEX_ALLOC_MEM and returns its pointer to reflog_expire_config(). The
> > function exits without freeing the memory:
> >
> > 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 freeing the entry pointer on exit of the
> > reflog_expire_config() function. This frees both the entry structure and
> > its embedded pattern array thanks to the use of FLEX_ALLOC_MEM.
> 
> Hmm, this can't be right, can it? The end of reflog_expire_config()
> looks like this:
> 
>         ent = find_cfg_ent(opts, pattern, pattern_len);
>         if (!ent)
>                 return -1;
>         switch (slot) {
>         case REFLOG_EXPIRE_TOTAL:
>                 ent->expire_total = expire;
>                 break;
>         case REFLOG_EXPIRE_UNREACH:
>                 ent->expire_unreachable = expire;
>                 break;
>         }
>         return 0;
> 
> So if we free(ent), then what was the point of the function? We'd set
> some fields in it and then throw it away?
> 
> And indeed, find_cfg_ent() seems to add the newly allocated entry to the
> list opt->entries list. So by freeing here, we're leaving a dangling
> pointer in that list.
> 
> Probably that list needs to be cleaned up when cmd_reflog_expire()
> finishes?
> 
> -Peff
Oh. Yep, you're right. My brain didn't quite process what was going on. Yea this definitely won't work.

Thanks, Jake

← back to recent threads