# Git crashes on pull 6 messages from 2009-09-15 to 2009-09-18. Participants: Guido Ostkamp, Junio C Hamano, Michael Wookey, Tay Ray Chuan. Thread: https://gitlist.dev/t/20958 ## Guido Ostkamp, 2009-09-15 18:47 Subject: Git crashes on pull Message-ID: URL: https://gitlist.dev/e/alpine.LSU.2.01.0909152044450.10936%40bianca.dialin.t-online.de ``` Hi, I have a clone of http://git.postgresql.org/git/postgresql.git where head is at commit 167501570c74390dfb7a5dd71e260ab3d4fd9904. I'm using Git version 1.6.5.rc1.10.g20f34 (should be at commit 20f34902d154f390ebaa7eed7f42ad14140b8acb from Mon Sep 14 10:49:01 2009 +0200) Now when I 'git pull' then Git crashes with git pull 2>&1 > /tmp/git-error *** glibc detected *** git-remote-curl: free(): invalid pointer: 0xb7d19140 *** ======= Backtrace: ========= /lib/libc.so.6[0xb7c4f4b6] /lib/libc.so.6(cfree+0x89)[0xb7c51179] git-remote-curl[0x804d290] git-remote-curl[0x804df04] git-remote-curl[0x8065ea5] git-remote-curl[0x804aac6] /lib/libc.so.6(__libc_start_main+0xe0)[0xb7bfefe0] git-remote-curl[0x804a991] ======= Memory map: ======== 08048000-080a1000 r-xp 00000000 08:15 1658246 /usr/local/libexec/git-core/git-remote-curl 080a1000-080a2000 r--p 00058000 08:15 1658246 /usr/local/libexec/git-core/git-remote-curl 080a2000-080a3000 rw-p 00059000 08:15 1658246 /usr/local/libexec/git-core/git-remote-curl 080a3000-08143000 rw-p 080a3000 00:00 0 [heap] b4400000-b4421000 rw-p b4400000 00:00 0 b4421000-b4500000 ---p b4421000 00:00 0 b45ea000-b45f4000 r-xp 00000000 08:13 1097821 /lib/libgcc_s.so.1 b45f4000-b45f6000 rw-p 00009000 08:13 1097821 /lib/libgcc_s.so.1 ... Any idea what's causing this? Please keep me on CC, as I'm not subscribed on list. Regards Guido ``` ## Junio C Hamano, 2009-09-15 19:22 Subject: Re: Git crashes on pull Message-ID: <7vljkg57xs.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vljkg57xs.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Guido Ostkamp writes: > I have a clone of http://git.postgresql.org/git/postgresql.git where > head is at commit 167501570c74390dfb7a5dd71e260ab3d4fd9904. > > I'm using Git version 1.6.5.rc1.10.g20f34 (should be at commit > 20f34902d154f390ebaa7eed7f42ad14140b8acb from Mon Sep 14 10:49:01 2009 > +0200) > > Now when I 'git pull' then Git crashes with > > git pull 2>&1 > /tmp/git-error > *** glibc detected *** git-remote-curl: free(): invalid pointer: Please try this patch, which I have been preparing for later pushout. From: Junio C Hamano Date: Mon, 14 Sep 2009 14:48:15 -0700 Subject: [PATCH] http.c: avoid freeing an uninitialized pointer An earlier 59b8d38 (http.c: remove verification of remote packs) left the variable "url" uninitialized; "goto cleanup" codepath can free it which is not very nice. Signed-off-by: Junio C Hamano --- http.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/http.c b/http.c index d0cc1b3..15926d8 100644 --- a/http.c +++ b/http.c @@ -866,7 +866,7 @@ static int fetch_pack_index(unsigned char *sha1, const char *base_url) int ret = 0; char *hex = xstrdup(sha1_to_hex(sha1)); char *filename; - char *url; + char *url = NULL; struct strbuf buf = STRBUF_INIT; if (has_pack_index(sha1)) { -- 1.6.5.rc1 ``` ## Guido Ostkamp, 2009-09-15 22:30 Subject: Re: Git crashes on pull Message-ID: URL: https://gitlist.dev/e/alpine.LSU.2.01.0909160022430.24554%40bianca.dialin.t-online.de In-Reply-To: <7vljkg57xs.fsf@alter.siamese.dyndns.org> ``` On Tue, 15 Sep 2009, Junio C Hamano wrote: > Please try this patch, which I have been preparing for later pushout. > > From: Junio C Hamano > Date: Mon, 14 Sep 2009 14:48:15 -0700 > Subject: [PATCH] http.c: avoid freeing an uninitialized pointer > > An earlier 59b8d38 (http.c: remove verification of remote packs) left > the variable "url" uninitialized; "goto cleanup" codepath can free it > which is not very nice. > > Signed-off-by: Junio C Hamano Appears to be working ok now, thanks. BTW: Is there any way to easily invoke GDB in case of such a problem to get a real symbolic stack backtrace? I tried it on the 'git' binary, but of course this didn't work because it invokes a git-pull script which again runs another git-remote-curl binary. Regards Guido ``` ## Junio C Hamano, 2009-09-15 22:54 Subject: Re: Git crashes on pull Message-ID: <7vzl8v4y5g.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vzl8v4y5g.fsf%40alter.siamese.dyndns.org In-Reply-To: ``` Guido Ostkamp writes: > On Tue, 15 Sep 2009, Junio C Hamano wrote: > >> Please try this patch, which I have been preparing for later pushout. >> >> From: Junio C Hamano >> Date: Mon, 14 Sep 2009 14:48:15 -0700 >> Subject: [PATCH] http.c: avoid freeing an uninitialized pointer >> >> An earlier 59b8d38 (http.c: remove verification of remote packs) left >> the variable "url" uninitialized; "goto cleanup" codepath can free it >> which is not very nice. >> >> Signed-off-by: Junio C Hamano > > Appears to be working ok now, thanks. Thanks. The sad part of the story was that this regression was introduced by a change to work around recent breakage observed when fetching from the http server github runs, and it was the primary purpose of pushing 1.6.4.3 out. Now we need to cut a 1.6.4.4 with this fix-on-fix soon, like tomorrow. > BTW: Is there any way to easily invoke GDB in case of such a problem > to get a real symbolic stack backtrace? > > I tried it on the 'git' binary, but of course this didn't work because > it invokes a git-pull script which again runs another git-remote-curl > binary. Not very easily. The best you can do is to run with GIT_TRACE to see what command actually dies and run that binary directly. gdb can choose to follow either parent or child across forks, but I do not know how to tell it to follow across execs into a different binary. ``` ## Michael Wookey, 2009-09-15 23:30 Subject: Re: Git crashes on pull Message-ID: URL: https://gitlist.dev/e/d2e97e800909151630w44d440f5hadb088aa5e1f8e22%40mail.gmail.com In-Reply-To: <7vzl8v4y5g.fsf@alter.siamese.dyndns.org> ``` 2009/9/16 Junio C Hamano : > Guido Ostkamp writes: > >> On Tue, 15 Sep 2009, Junio C Hamano wrote: >> >>> Please try this patch, which I have been preparing for later pushout. >>> >>> From: Junio C Hamano >>> Date: Mon, 14 Sep 2009 14:48:15 -0700 >>> Subject: [PATCH] http.c: avoid freeing an uninitialized pointer >>> >>> An earlier 59b8d38 (http.c: remove verification of remote packs) left >>> the variable "url" uninitialized; "goto cleanup" codepath can free it >>> which is not very nice. >>> >>> Signed-off-by: Junio C Hamano >> >> Appears to be working ok now, thanks. > > Thanks. > > The sad part of the story was that this regression was introduced by a > change to work around recent breakage observed when fetching from the http > server github runs, and it was the primary purpose of pushing 1.6.4.3 out. If only I had given it a run with the clang static analyzer earlier :( Here is what Xcode would have shown - http://dl.getdropbox.com/u/1006983/git-clang.png I can make the Xcode project available if anyone is interested. ``` ## Tay Ray Chuan, 2009-09-18 13:39 Subject: Re: Git crashes on pull Message-ID: <20090918213948.4bb65f4e.rctay89@gmail.com> URL: https://gitlist.dev/e/20090918213948.4bb65f4e.rctay89%40gmail.com In-Reply-To: <7vzl8v4y5g.fsf@alter.siamese.dyndns.org> ``` Hi, On Wed, Sep 16, 2009 at 6:54 AM, Junio C Hamano wrote: > Thanks. > > The sad part of the story was that this regression was introduced by a > change to work around recent breakage observed when fetching from the http > server github runs, and it was the primary purpose of pushing 1.6.4.3 out. > > Now we need to cut a 1.6.4.4 with this fix-on-fix soon, like tomorrow. sorry for all the trouble caused. Junio, do you think moving out the free() would be a better option? Setting it to NULL just so we can free() is rather contrived, I feel. -- >8 -- Subject: [PATCH] http.c: move free() out of cleanup block Instead of initializing a variable (url) just so we can do a free() on it, as in b202514 (http.c: avoid freeing an uninitialized pointer), we move the free() out of cleanup block. Signed-off-by: Tay Ray Chuan --- http.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/http.c b/http.c index 23b2a19..a67f62e 100644 --- a/http.c +++ b/http.c @@ -866,7 +866,7 @@ static int fetch_pack_index(unsigned char *sha1, const char *base_url) int ret = 0; char *hex = xstrdup(sha1_to_hex(sha1)); char *filename; - char *url = NULL; + char *url; struct strbuf buf = STRBUF_INIT; if (has_pack_index(sha1)) { @@ -885,9 +885,9 @@ static int fetch_pack_index(unsigned char *sha1, const char *base_url) if (http_get_file(url, filename, 0) != HTTP_OK) ret = error("Unable to get pack index %s\n", url); + free(url); cleanup: free(hex); - free(url); return ret; } -- 1.6.4.2 ```