From 80ae35227d566977ad21eb6e35f49e1ca5d5a940 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 19 Dec 2024 12:50:18 +0100 Subject: [PATCH 01/12] load_one_loose_object_map(): fix resource leak Pointed out by Coverity. While at it, reduce near-duplicate clean-up code at the end of the function. Signed-off-by: Johannes Schindelin --- loose.c | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/loose.c b/loose.c index 0b626c1b854642..940a9e0dfee879 100644 --- a/loose.c +++ b/loose.c @@ -65,6 +65,7 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_ { struct strbuf buf = STRBUF_INIT, path = STRBUF_INIT; FILE *fp; + int ret = -1; if (!loose->map) loose_object_map_init(&loose->map); @@ -84,7 +85,6 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_ return 0; } - errno = 0; if (strbuf_getwholeline(&buf, fp, '\n') || strcmp(buf.buf, loose_object_header)) goto err; while (!strbuf_getline_lf(&buf, fp)) { @@ -98,13 +98,12 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_ insert_loose_map(loose, &oid, &compat_oid); } - strbuf_release(&buf); - strbuf_release(&path); - return errno ? -1 : 0; + ret = ferror(fp) ? -1 : 0; err: + fclose(fp); strbuf_release(&buf); strbuf_release(&path); - return -1; + return ret; } int repo_read_loose_object_map(struct repository *repo) From 546a7c5d9f76fc9cb71305bd0bcb4bc7693ecb39 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 5 May 2026 02:26:58 +0200 Subject: [PATCH 02/12] loose: avoid closing invalid fd on error path `write_one_object()` opens a file at line 186 and jumps to the errout label on failure. The errout cleanup unconditionally calls `close(fd)`, but when `open()` itself failed, fd is -1. Calling `close(-1)` is harmless on most platforms (returns EBADF) but is undefined behavior per POSIX and can confuse fd tracking in sanitizer builds. Guard the close with fd >= 0. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin --- loose.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/loose.c b/loose.c index 940a9e0dfee879..bf01d3e42def34 100644 --- a/loose.c +++ b/loose.c @@ -201,7 +201,8 @@ static int write_one_object(struct odb_source_loose *loose, return 0; errout: error_errno(_("failed to write loose object index %s"), path.buf); - close(fd); + if (fd >= 0) + close(fd); rollback_lock_file(&lock); strbuf_release(&buf); strbuf_release(&path); From 17c3b4ce4f0051bf0c27ae157c25af97275ba742 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Thu, 19 Dec 2024 16:55:16 +0100 Subject: [PATCH 03/12] download_https_uri_to_file(): do not leak fd upon failure When the `git-remote-https` command fails, we do not want to leak `child_out`. Pointed out by Coverity. Signed-off-by: Johannes Schindelin --- bundle-uri.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle-uri.c b/bundle-uri.c index 3b2e347288c3b7..34fa452e760905 100644 --- a/bundle-uri.c +++ b/bundle-uri.c @@ -378,7 +378,7 @@ static int download_https_uri_to_file(const char *file, const char *uri) if (child_in) fclose(child_in); if (finish_command(&cp)) - return 1; + result = 1; if (child_out) fclose(child_out); return result; From 0360016d91bfca251c914e46700ba190798e1911 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 27 Jun 2026 17:57:07 +0200 Subject: [PATCH 04/12] run-command: avoid `close(-1)` in `start_command()` error paths When `start_command()` fails to set up a pipe partway through, it rolls back by closing the pipe ends it has already opened. For descriptors supplied by the caller rather than allocated locally, that rollback tested `if (cmd->in)` / `if (cmd->out)` before calling close(). The CHILD_PROCESS_INIT default of -1 ("no descriptor") is non-zero and so passes the test, meaning a caller that sets cmd->no_stdin or cmd->no_stdout without supplying a real fd ends up triggering close(-1) on the error path. The stdin-pipe failure branch a few lines above already uses the right idiom, `if (cmd->out > 0)`, which rejects both the -1 sentinel and 0 (the parent's own standard streams). Apply it to the three remaining rollback sites. Reported by Coverity as CID 1049722 ("Argument cannot be negative"). Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin --- run-command.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/run-command.c b/run-command.c index e70a8a387b9042..ce84db87824262 100644 --- a/run-command.c +++ b/run-command.c @@ -706,7 +706,7 @@ int start_command(struct child_process *cmd) failed_errno = errno; if (need_in) close_pair(fdin); - else if (cmd->in) + else if (cmd->in > 0) close(cmd->in); str = "standard output"; goto fail_pipe; @@ -720,11 +720,11 @@ int start_command(struct child_process *cmd) failed_errno = errno; if (need_in) close_pair(fdin); - else if (cmd->in) + else if (cmd->in > 0) close(cmd->in); if (need_out) close_pair(fdout); - else if (cmd->out) + else if (cmd->out > 0) close(cmd->out); str = "standard error"; fail_pipe: From 8c623cc28f5c86b33b03deec0c2d0b5486b08c02 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 19 Apr 2026 08:42:00 +0200 Subject: [PATCH 05/12] line-log: avoid redundant copy that leaks in process_ranges When `bloom_filter_check()` indicates that a commit does not touch any of the tracked paths, `line_log_process_ranges_arbitrary_commit()` propagates the current ranges to the parent by calling `line_log_data_copy()` and passing the copy to add_line_range(). However, `add_line_range()` always makes its own copy internally (via line_log_data_copy or line_log_data_merge), so the caller's copy is never freed and leaks every time this path is taken. Pass range directly to `add_line_range()` instead of making a redundant intermediate copy. The callee's internal copy handles ownership correctly. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin --- line-log.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/line-log.c b/line-log.c index 5fc75ae275e03a..0179f138f70288 100644 --- a/line-log.c +++ b/line-log.c @@ -1141,8 +1141,7 @@ int line_log_process_ranges_arbitrary_commit(struct rev_info *rev, struct commit if (range) { if (commit->parents && !bloom_filter_check(rev, commit, range)) { - struct line_log_data *prange = line_log_data_copy(range); - add_line_range(rev, commit->parents->item, prange); + add_line_range(rev, commit->parents->item, range); clear_commit_line_range(rev, commit); } else if (commit->parents && commit->parents->next) changed = process_ranges_merge_commit(rev, commit, range); From 8a8fe2d3e342b912de1013082ac838e8544d7031 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 19 Apr 2026 08:42:21 +0200 Subject: [PATCH 06/12] dir: free allocations on parse-error paths in `read_one_dir()` Two of `read_one_dir()`'s parse-error early returns leak ud.untracked and ud.dirs. Plug them. The other early returns in the same function are fine: they occur after the `xmalloc()`+`memcpy()` that copies ud into `*untracked_`, at which point ownership is transferred to the caller. `read_untracked_extension()` then releases everything via `free_untracked_cache()` on failure. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin --- dir.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/dir.c b/dir.c index 32430090dcdf26..23335b9f7ae12b 100644 --- a/dir.c +++ b/dir.c @@ -3792,13 +3792,18 @@ static int read_one_dir(struct untracked_cache_dir **untracked_, ALLOC_ARRAY(ud.untracked, ud.untracked_nr); ud.dirs_alloc = ud.dirs_nr = decode_varint(&data); - if (data > end) + if (data > end) { + free(ud.untracked); return -1; + } ALLOC_ARRAY(ud.dirs, ud.dirs_nr); eos = memchr(data, '\0', end - data); - if (!eos || eos == end) + if (!eos || eos == end) { + free(ud.untracked); + free(ud.dirs); return -1; + } *untracked_ = untracked = xmalloc(st_add3(sizeof(*untracked), eos - data, 1)); memcpy(untracked, &ud, sizeof(ud)); From 5397ea785c6da50e977598a35d03af82cb2a5e4d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 27 Apr 2026 23:15:11 +0200 Subject: [PATCH 07/12] submodule: fix cwd leak in `get_superproject_working_tree()` `get_superproject_working_tree()` allocates cwd via `xgetcwd()` at the top of the function, but two early-return paths (when not inside a work tree, and when strbuf_realpath for "../" fails) return 0 without freeing it. Redirect these early returns through a cleanup label that frees cwd before returning. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin --- submodule.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/submodule.c b/submodule.c index fd91201a92d7b0..92dfb0fc2d341f 100644 --- a/submodule.c +++ b/submodule.c @@ -2627,13 +2627,12 @@ int get_superproject_working_tree(struct strbuf *buf) * We might have a superproject, but it is harder * to determine. */ - return 0; + goto out; if (!strbuf_realpath(&one_up, "../", 0)) - return 0; + goto out; subpath = relative_path(cwd, one_up.buf, &sb); - strbuf_release(&one_up); prepare_submodule_repo_env(&cp.env); strvec_pop(&cp.env); @@ -2678,20 +2677,22 @@ int get_superproject_working_tree(struct strbuf *buf) ret = 1; free(super_wt); } - free(cwd); - strbuf_release(&sb); code = finish_command(&cp); if (code == 128) /* '../' is not a git repository */ - return 0; - if (code == 0 && len == 0) + ret = 0; + else if (code == 0 && len == 0) /* There is an unrelated git repository at '../' */ - return 0; - if (code) + ret = 0; + else if (code) die(_("ls-tree returned unexpected return code %d"), code); +out: + strbuf_release(&sb); + strbuf_release(&one_up); + free(cwd); return ret; } From 0048c0ca2752853dfba7ae1bf89dd70c8e501d54 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Mon, 27 Apr 2026 23:41:47 +0200 Subject: [PATCH 08/12] worktree: fix resource leaks when branch creation fails In the "add" subcommand, when `run_command()` fails while creating a new branch (line 948), the function returns -1 immediately without freeing the allocations made earlier: path (from prefix_filename at line 858), opt_track, branch_to_free, and new_branch_to_free. Redirect the error return through the existing cleanup block at the end of the function so all four allocations are properly freed. Pointed out by Coverity. Assisted-by: Claude Opus 4.6 Signed-off-by: Johannes Schindelin --- builtin/worktree.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index d21c43fde38b5e..4bc7b4f6e7199a 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -945,14 +945,17 @@ static int add(int ac, const char **av, const char *prefix, strvec_push(&cp.args, branch); if (opt_track) strvec_push(&cp.args, opt_track); - if (run_command(&cp)) - return -1; + if (run_command(&cp)) { + ret = -1; + goto cleanup; + } branch = new_branch; } else if (opt_track) { die(_("--[no-]track can only be used if a new branch is created")); } ret = add_worktree(path, branch, &opts); +cleanup: free(path); free(opt_track); free(branch_to_free); From 4048a225a5c3c0b698a2dbc58c756d217f851a72 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 27 Jun 2026 18:35:30 +0200 Subject: [PATCH 09/12] imap-send: avoid leaking the IMAP upload buffer When uploading messages via libcurl, `curl_append_msgs_to_imap()` accumulates each one in a strbuf that grows across loop iterations but is never released before the function returns. Release it alongside the existing libcurl cleanup. Reported by Coverity as CID 1671507 ("Resource leak"). Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin --- imap-send.c | 1 + 1 file changed, 1 insertion(+) diff --git a/imap-send.c b/imap-send.c index cfd6a5120c50e4..0d16d02029232b 100644 --- a/imap-send.c +++ b/imap-send.c @@ -1750,6 +1750,7 @@ static int curl_append_msgs_to_imap(struct imap_server_conf *server, curl_easy_cleanup(curl); curl_global_cleanup(); + strbuf_release(&msgbuf.buf); if (cred.username) { if (res == CURLE_OK) From 13ecebcdee633689b861418a005cf4f64c190fb3 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 27 Jun 2026 18:57:05 +0200 Subject: [PATCH 10/12] reftable/table: release filter on error path `reftable_table_refs_for_unindexed()` allocates a filtering_ref_iterator and then calls `reftable_buf_add()` to populate its oid buffer. On success ownership is transferred to the output iterator, but if `reftable_buf_add()` fails, the goto-out cleanup only frees the table iterator and walks away from both the filter allocation and the oid buffer that `reftable_buf_add()` may have grown. Release filter->oid and free filter alongside the existing table iterator cleanup. Reported by Coverity as CID 1671512 ("Resource leak"). Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin --- reftable/table.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/reftable/table.c b/reftable/table.c index 56362df0eda5a6..d604ddebf44dda 100644 --- a/reftable/table.c +++ b/reftable/table.c @@ -709,6 +709,10 @@ static int reftable_table_refs_for_unindexed(struct reftable_table *t, if (ti) table_iter_close(ti); reftable_free(ti); + if (filter) { + reftable_buf_release(&filter->oid); + reftable_free(filter); + } } return err; } From 97049d7cc3960937d822fb9403849d1dba063b78 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sun, 28 Jun 2026 12:18:04 +0200 Subject: [PATCH 11/12] fsmonitor: plug token-data leak on early daemon-startup failures `fsmonitor_run_daemon()` allocates `state.current_token_data` before any subordinate setup step that may fail (alias resolution, listener/health constructors, asynchronous IPC server init). On the successful path the listener thread takes ownership and clears the field during its teardown, so the `done:` cleanup block sees a NULL pointer. On every early-error path, however, control jumps straight to `done:` with the freshly allocated token data still referenced, and it is never freed, as Coverity flagged. Free it at the top of `done:` and clear the pointer. The success path is a no-op (the pointer is already NULL there); the error paths now drop the otherwise-leaked allocation. `fsmonitor_free_token_data()` is NULL-safe and asserts `client_ref_count == 0`, which holds trivially here because the IPC server has not yet begun accepting clients when these failures occur. Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin --- builtin/fsmonitor--daemon.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/builtin/fsmonitor--daemon.c b/builtin/fsmonitor--daemon.c index f920cf3a8202f6..4161dd82825b4c 100644 --- a/builtin/fsmonitor--daemon.c +++ b/builtin/fsmonitor--daemon.c @@ -1418,6 +1418,8 @@ static int fsmonitor_run_daemon(void) err = fsmonitor_run_daemon_1(&state); done: + fsmonitor_free_token_data(state.current_token_data); + state.current_token_data = NULL; pthread_cond_destroy(&state.cookies_cond); pthread_mutex_destroy(&state.main_lock); { From a5a6c27184097f0f8bfc1174e691dd1b94eb165d Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Sat, 27 Jun 2026 17:26:42 +0200 Subject: [PATCH 12/12] mingw: make `exit_process()` own the process handle on all paths After "mingw: kill child processes in a gentler way", the ownership of the HANDLE passed to `exit_process()` and `terminate_process_tree()` is inconsistent. `terminate_process_tree()` always closes the handle; `exit_process()` closes it on success and on the terminate-tree fallback, but leaks it on the early return where GetExitCodeProcess() fails or reports the process is no longer STILL_ACTIVE. `mingw_kill()` compensated by closing the handle on its own error path, which is a double-close on every error path that does not hit that one leaky branch -- the callee has already closed the handle by then. Coverity flagged the resulting use-after-free as CID 1437238. Pin down the invariant that `exit_process()` and `terminate_process_tree()` own the handle from the call onward and close it on every return path; with that, the bogus close in `mingw_kill()` goes away. Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin --- compat/mingw.c | 4 +--- compat/win32/exit-process.h | 1 + 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/compat/mingw.c b/compat/mingw.c index 41e055f7de885e..e2cb92a4144b36 100644 --- a/compat/mingw.c +++ b/compat/mingw.c @@ -2269,10 +2269,8 @@ int mingw_kill(pid_t pid, int sig) } ret = terminate_process_tree(h, 128 + sig); } - if (ret) { + if (ret) errno = err_win_to_posix(GetLastError()); - CloseHandle(h); - } return ret; } else if (pid > 0 && sig == 0) { HANDLE h = OpenProcess(PROCESS_QUERY_INFORMATION, FALSE, pid); diff --git a/compat/win32/exit-process.h b/compat/win32/exit-process.h index d53989884cfb0c..26004161bcbdc3 100644 --- a/compat/win32/exit-process.h +++ b/compat/win32/exit-process.h @@ -159,6 +159,7 @@ static int exit_process(HANDLE process, int exit_code) return terminate_process_tree(process, exit_code); } + CloseHandle(process); return 0; }