From: Johannes Schindelin Date: Sun, 5 Jul 2026 08:24:21 +0000 (+0000) Subject: run-command: avoid `close(-1)` in `start_command()` error paths X-Git-Url: http://git.ipfire.org/gitweb/index.cgi?a=commitdiff_plain;h=c3f89beb733a260866ec9c163615bce59e77481b;p=thirdparty%2Fgit.git 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 Signed-off-by: Junio C Hamano --- diff --git a/run-command.c b/run-command.c index e70a8a387b..ce84db8782 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: