summaryrefslogtreecommitdiff
path: root/sub-process.c
diff options
context:
space:
mode:
authorJohannes Sixt <j6t@kdbg.org>2017-10-03 20:24:57 (GMT)
committerJunio C Hamano <gitster@pobox.com>2017-10-04 04:58:15 (GMT)
commit2944a94c6b74d3941f63d1f4eee5bdfbbf5cd400 (patch)
treedc4bd33e361edb1ad34b9e665fe8169a1450ba27 /sub-process.c
parent51bfb734df43dc2d9ddbc7234a8723a7b1cfb322 (diff)
downloadgit-2944a94c6b74d3941f63d1f4eee5bdfbbf5cd400.zip
git-2944a94c6b74d3941f63d1f4eee5bdfbbf5cd400.tar.gz
git-2944a94c6b74d3941f63d1f4eee5bdfbbf5cd400.tar.bz2
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 <t.gummerer@gmail.com> Signed-off-by: Johannes Sixt <j6t@kdbg.org> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'sub-process.c')
-rw-r--r--sub-process.c3
1 files changed, 1 insertions, 2 deletions
diff --git a/sub-process.c b/sub-process.c
index fcc4832..648b3a3 100644
--- a/sub-process.c
+++ b/sub-process.c
@@ -74,13 +74,12 @@ int subprocess_start(struct hashmap *hashmap, struct subprocess_entry *entry, co
{
int err;
struct child_process *process;
- const char *argv[] = { cmd, NULL };
entry->cmd = cmd;
process = &entry->process;
child_process_init(process);
- process->argv = argv;
+ argv_array_push(&process->args, cmd);
process->use_shell = 1;
process->in = -1;
process->out = -1;