git-fetch segfault in git 1.5.5.1

git-fetch segfault in git 1.5.5.1

[Dave Jones]

Since master.kernel.org updated to latest, I noticed that I could crash
git-fetch by doing this..

export KERNEL=/pub/scm/linux/kernel/git/
git fetch $KERNEL/torvalds/linux-2.6 master:linus

(gdb) bt
#0  0x000000349fd6d44b in free () from /lib64/libc.so.6
#1  0x000000000048f4eb in transport_unlock_pack (transport=0x7ce530) at transport.c:811
#2  0x000000349fd31b25 in exit () from /lib64/libc.so.6
#3  0x00000000004043d8 in handle_internal_command (argc=3, argv=0x7fffea4449f0) at git.c:379
#4  0x0000000000404547 in main (argc=3, argv=0x7fffea4449f0) at git.c:443
#5  0x000000349fd1c784 in __libc_start_main () from /lib64/libc.so.6
#6  0x0000000000403ef9 in ?? ()
#7  0x00007fffea4449d8 in ?? ()
#8  0x0000000000000000 in ?? ()

I then remembered, my .bashrc has this..

export MALLOC_PERTURB_=$(($RANDOM % 255 + 1))

which is handy for showing up such bugs.

More info on this glibc feature is at http://udrepper.livejournal.com/11429.html

	Dave

-- 
http://www.codemonkey.org.uk

[PATCH] Fix use after free() in builtin-fetch

[Alex Riesen]

As reported by Dave Jones:

Since master.kernel.org updated to latest, I noticed that I could crash
git-fetch by doing this..

export KERNEL=/pub/scm/linux/kernel/git/
git fetch $KERNEL/torvalds/linux-2.6 master:linus

(gdb) bt
 0  0x000000349fd6d44b in free () from /lib64/libc.so.6
 1  0x000000000048f4eb in transport_unlock_pack (transport=0x7ce530) at transport.c:811
 2  0x000000349fd31b25 in exit () from /lib64/libc.so.6
 3  0x00000000004043d8 in handle_internal_command (argc=3, argv=0x7fffea4449f0) at git.c:379
 4  0x0000000000404547 in main (argc=3, argv=0x7fffea4449f0) at git.c:443
 5  0x000000349fd1c784 in __libc_start_main () from /lib64/libc.so.6
 6  0x0000000000403ef9 in ?? ()
 7  0x00007fffea4449d8 in ?? ()
 8  0x0000000000000000 in ?? ()

I then remembered, my .bashrc has this..

export MALLOC_PERTURB_=$(($RANDOM % 255 + 1))

which is handy for showing up such bugs.

More info on this glibc feature is at http://udrepper.livejournal.com/11429.html

Signed-off-by: Alex Riesen <raa.lkml@gmail.com>
---
Dave Jones, Mon, Apr 28, 2008 20:41:38 +0200:
> (gdb) bt
> #0  0x000000349fd6d44b in free () from /lib64/libc.so.6
> #1  0x000000000048f4eb in transport_unlock_pack (transport=0x7ce530) at transport.c:811
> #2  0x000000349fd31b25 in exit () from /lib64/libc.so.6

atexit strikes again. Besides, I believe, do_fetch has no bussiness in
deallocation of resources it did not allocate.

 builtin-fetch.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/builtin-fetch.c b/builtin-fetch.c
index 139a6b1..167f948 100644
--- a/builtin-fetch.c
+++ b/builtin-fetch.c
@@ -577,8 +577,6 @@ static int do_fetch(struct transport *transport,
 		free_refs(ref_map);
 	}
 
-	transport_disconnect(transport);
-
 	return 0;
 }
 
@@ -599,6 +597,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 	int i;
 	static const char **refs = NULL;
 	int ref_nr = 0;
+	int exit_code;
 
 	/* Record the command line for the reflog */
 	strbuf_addstr(&default_rla, "fetch");
@@ -652,6 +651,9 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
 
 	signal(SIGINT, unlock_pack_on_signal);
 	atexit(unlock_pack);
-	return do_fetch(transport,
+	exit_code = do_fetch(transport,
 			parse_fetch_refspec(ref_nr, refs), ref_nr);
+	transport_disconnect(transport);
+	transport = NULL;
+	return exit_code;
 }
-- 
1.5.5.1.118.g6dd1b6.dirty

Re: [PATCH] Fix use after free() in builtin-fetch

[Junio C Hamano]

Alex Riesen <raa.lkml@gmail.com> writes:

> As reported by Dave Jones:
> ...
> export MALLOC_PERTURB_=$(($RANDOM % 255 + 1))
>
> which is handy for showing up such bugs.
>
> More info on this glibc feature is at http://udrepper.livejournal.com/11429.html
>
> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>

Thanks.  I can reproduce the issue and the fix (and can even bisect this
down to ba22785 (Reduce the number of connects when fetching, 2008-02-04),
which makes perfect sense).

Will apply to 'maint' and merge up.