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==
Fix this by allocating the memory on properly on the heap. This memory
is allocated on the heap, and never free'd. However the same seems to be
true for struct child_process, so it should be fine to just let the
memory be free'd when the process terminates.
Signed-off-by: Thomas Gummerer <t.gummerer@xxxxxxxxx>
---
sub-process.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/sub-process.c b/sub-process.c
index 6dde5062be..4680af8193 100644
--- a/sub-process.c
+++ b/sub-process.c
@@ -77,7 +77,9 @@ int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, co
{
int err;
struct child_process *process;
- const char *argv[] = { cmd, NULL };
+ const char **argv = xmalloc(2 * sizeof(char *));
+ argv[0] = cmd;
+ argv[1] = NULL;
entry->cmd = cmd;
process = &entry->process;
--
2.14.1.480.gb18f417b89
