Skip to content

Commit 2944a94

Browse files
j6tgitster
authored andcommitted
sub-process: use child_process.args instead of child_process.argv
Currently the argv is only allocated on the stack, and then assigned to process->argv. When the start_subprocess function goes out of scope, the local argv variable is eliminated from the stack, but the pointer is still kept around in process->argv. Much later when we try to access the same process->argv in finish_command, this leads us to access a memory location that no longer contains what we want. As argv0 is only used for printing errors, this is not easily noticed in normal git operations. However when running t0021-conversion.sh through valgrind, valgrind rightfully complains: ==21024== Invalid read of size 8 ==21024== at 0x2ACF64: finish_command (run-command.c:869) ==21024== by 0x2D6B18: subprocess_exit_handler (sub-process.c:72) ==21024== by 0x2AB41E: cleanup_children (run-command.c:45) ==21024== by 0x2AB526: cleanup_children_on_exit (run-command.c:81) ==21024== by 0x54AD487: __run_exit_handlers (in /usr/lib/libc-2.26.so) ==21024== by 0x54AD4D9: exit (in /usr/lib/libc-2.26.so) ==21024== by 0x11A9EF: handle_builtin (git.c:550) ==21024== by 0x11ABCC: run_argv (git.c:602) ==21024== by 0x11AD8E: cmd_main (git.c:679) ==21024== by 0x1BF125: main (common-main.c:43) ==21024== Address 0x1ffeffec00 is on thread 1's stack ==21024== 1504 bytes below stack pointer ==21024== These days, the child_process structure has its own args array, and the standard way to set up its argv[] is to use that one, instead of assigning to process->argv to point at an array that is outside. Use that facility automatically fixes this issue. Reported-by: Thomas Gummerer <[email protected]> Signed-off-by: Johannes Sixt <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 51bfb73 commit 2944a94

File tree

1 file changed

+1
-2
lines changed

1 file changed

+1
-2
lines changed

sub-process.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,12 @@ int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, co
7474
{
7575
int err;
7676
struct child_process *process;
77-
const char *argv[] = { cmd, NULL };
7877

7978
entry->cmd = cmd;
8079
process = &entry->process;
8180

8281
child_process_init(process);
83-
process->argv = argv;
82+
argv_array_push(&process->args, cmd);
8483
process->use_shell = 1;
8584
process->in = -1;
8685
process->out = -1;

0 commit comments

Comments
 (0)