Segmentation fault on http clone, post-1.5.6

Segmentation fault on http clone, post-1.5.6

[Teemu Likonen]

With the current "master" branch version (29b0d0191) I get segmentation
fault when trying to clone a git repo with http protocol. Tried a couple
of times and it's always reproducible. You can test with the following
repository (about 5.5 MB):

  git clone http://www.iki.fi/tlikonen/voikko.git

I also build git from the tag v1.5.6 and it seems to work fine, so
I guess the bug was introduced after 1.5.6.

Re: Segmentation fault on http clone, post-1.5.6

[Jeff King]

On Tue, Jun 24, 2008 at 04:04:57PM +0300, Teemu Likonen wrote:

> With the current "master" branch version (29b0d0191) I get segmentation
> fault when trying to clone a git repo with http protocol. Tried a couple
> of times and it's always reproducible. You can test with the following
> repository (about 5.5 MB):
> 
>   git clone http://www.iki.fi/tlikonen/voikko.git

I can't reproduce the segfault here.

> I also build git from the tag v1.5.6 and it seems to work fine, so
> I guess the bug was introduced after 1.5.6.

That sounds like an excellent opportunity to learn about git-bisect. Can
you try bisecting the bug and reporting back the problematic commit?

-Peff

Re: Segmentation fault on http clone, post-1.5.6

[Teemu Likonen]

Jeff King wrote (2008-06-24 12:40 -0400):

> On Tue, Jun 24, 2008 at 04:04:57PM +0300, Teemu Likonen wrote:
> 
> > With the current "master" branch version (29b0d0191) I get
> > segmentation fault when trying to clone a git repo with http
> > protocol. Tried a couple of times and it's always reproducible. You
> > can test with the following repository (about 5.5 MB):
> > 
> >   git clone http://www.iki.fi/tlikonen/voikko.git
> 
> I can't reproduce the segfault here.
> 
> > I also build git from the tag v1.5.6 and it seems to work fine, so
> > I guess the bug was introduced after 1.5.6.
> 
> That sounds like an excellent opportunity to learn about git-bisect.
> Can you try bisecting the bug and reporting back the problematic
> commit?

Indeed. I have now officially bisected the problem and the first bad or
problematic commit is 8eca0b47 "implement some resilience against pack
corruptions" (hence Cc to Nicolas, the author). This is always
reproducible in my Debian 4.0 box.

Re: Segmentation fault on http clone, post-1.5.6

[Mike Hommey]

On Tue, Jun 24, 2008 at 09:57:23PM +0300, Teemu Likonen wrote:
> Jeff King wrote (2008-06-24 12:40 -0400):
> 
> > On Tue, Jun 24, 2008 at 04:04:57PM +0300, Teemu Likonen wrote:
> > 
> > > With the current "master" branch version (29b0d0191) I get
> > > segmentation fault when trying to clone a git repo with http
> > > protocol. Tried a couple of times and it's always reproducible. You
> > > can test with the following repository (about 5.5 MB):
> > > 
> > >   git clone http://www.iki.fi/tlikonen/voikko.git
> > 
> > I can't reproduce the segfault here.
> > 
> > > I also build git from the tag v1.5.6 and it seems to work fine, so
> > > I guess the bug was introduced after 1.5.6.
> > 
> > That sounds like an excellent opportunity to learn about git-bisect.
> > Can you try bisecting the bug and reporting back the problematic
> > commit?
> 
> Indeed. I have now officially bisected the problem and the first bad or
> problematic commit is 8eca0b47 "implement some resilience against pack
> corruptions" (hence Cc to Nicolas, the author). This is always
> reproducible in my Debian 4.0 box.

Does it crash in verify_packfile() ?

Mike

Re: Segmentation fault on http clone, post-1.5.6

[Nicolas Pitre]

On Tue, 24 Jun 2008, Teemu Likonen wrote:

> Jeff King wrote (2008-06-24 12:40 -0400):
> 
> > On Tue, Jun 24, 2008 at 04:04:57PM +0300, Teemu Likonen wrote:
> > 
> > > With the current "master" branch version (29b0d0191) I get
> > > segmentation fault when trying to clone a git repo with http
> > > protocol. Tried a couple of times and it's always reproducible. You
> > > can test with the following repository (about 5.5 MB):
> > > 
> > >   git clone http://www.iki.fi/tlikonen/voikko.git
> > 
> > I can't reproduce the segfault here.
> > 
> > > I also build git from the tag v1.5.6 and it seems to work fine, so
> > > I guess the bug was introduced after 1.5.6.
> > 
> > That sounds like an excellent opportunity to learn about git-bisect.
> > Can you try bisecting the bug and reporting back the problematic
> > commit?
> 
> Indeed. I have now officially bisected the problem and the first bad or
> problematic commit is 8eca0b47 "implement some resilience against pack
> corruptions" (hence Cc to Nicolas, the author). This is always
> reproducible in my Debian 4.0 box.

I'm trying to reproduce your segfault with current master 
(v1.5.6-56-g29b0d01) but I just can't.

Can you provide a gdb backtrace of the segfault?  What my patch does is 
to return NULL in all cases when an object exists but can't be read 
instead of dying.  So if some user of read_sha1_file() is not checking 
for a returned null pointer then a null pointer dereference is most 
likely.

Note that read_sha1_file() could have returned NULL even before my 
patch, but not necessarily in all cases.


Nicolas

Re: Segmentation fault on http clone, post-1.5.6

[Teemu Likonen]

Nicolas Pitre wrote (2008-06-24 15:34 -0400):

> I'm trying to reproduce your segfault with current master
> (v1.5.6-56-g29b0d01) but I just can't.
> 
> Can you provide a gdb backtrace of the segfault?

Let's hope I'm doing this right. Just installed gdb for the first time.
I started with "gdb git" and then typed "run clone http://...".
Eventually it gave this:


Program received signal SIGSEGV, Segmentation fault.
0x080cb69a in find_pack_entry (sha1=0x81675bc "\224�d�BX\006�\020��\016�2\214\002\n�R", e=0xbfcccd18, ignore_packed=0x0) at cache.h:489
489             return memcmp(sha1, sha2, 20);


And "bt" command prints this:


#0  0x080cb69a in find_pack_entry (sha1=0x81675bc "\224�d�BX\006�\020��\016�2\214\002\n�R", e=0xbfcccd18, ignore_packed=0x0)
    at cache.h:489
#1  0x080cd05f in read_packed_sha1 (sha1=0x6900736b <Address 0x6900736b out of bounds>, type=0xbfccede4, size=0xbfccede8)
    at sha1_file.c:1924
#2  0x080cd211 in read_sha1_file (sha1=0x81675bc "\224�d�BX\006�\020��\016�2\214\002\n�R", type=0xbfccede4, size=0xbfccede8)
    at sha1_file.c:2016
#3  0x080b468a in parse_object (sha1=0x81675bc "\224�d�BX\006�\020��\016�2\214\002\n�R") at object.c:190
#4  0x080d69bd in walker_fetch (walker=0x8147610, targets=89, target=0x8163270, write_ref=0x0, write_ref_log_details=0x0) at walker.c:182
#5  0x080d16d1 in fetch_objs_via_curl (transport=0x81475e0, nr_objs=89, to_fetch=0x81630a0) at transport.c:369
#6  0x080d0c57 in transport_fetch_refs (transport=0x81475e0, refs=0x8163020) at transport.c:814
#7  0x0805be15 in cmd_clone (argc=2, argv=dwarf2_read_address: Corrupted DWARF expression.
) at builtin-clone.c:465
#8  0x0804b081 in handle_internal_command (argc=2, argv=0xbfcd1568) at git.c:252
#9  0x0804b272 in main (argc=0, argv=0x8165240) at git.c:448

Re: Segmentation fault on http clone, post-1.5.6

[Nicolas Pitre]

On Tue, 24 Jun 2008, Teemu Likonen wrote:

> Nicolas Pitre wrote (2008-06-24 15:34 -0400):
> 
> > I'm trying to reproduce your segfault with current master
> > (v1.5.6-56-g29b0d01) but I just can't.
> > 
> > Can you provide a gdb backtrace of the segfault?
> 
> Let's hope I'm doing this right. Just installed gdb for the first time.
> I started with "gdb git" and then typed "run clone http://...".
> Eventually it gave this:

Excellent!

The problem is probably fixed with this:

diff --git a/sha1_file.c b/sha1_file.c
index a92f023..b7d1a82 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -844,6 +844,8 @@ struct packed_git *parse_pack_index(unsigned char *sha1)
 
 	strcpy(p->pack_name, path);
 	p->pack_size = 0;
+	p->num_bad_objects = 0;
+	p->bad_object_sha1 = NULL;
 	p->next = NULL;
 	p->windows = NULL;
 	p->pack_fd = -1;

Could you confirm it?

However I just don't like the fact that pack structures are allocated 
and initialized in two places, which makes it error prone as 
demonstrated here.  So I'll cook up a better patch that fixes the 
duplication issue.


Nicolas

Re: Segmentation fault on http clone, post-1.5.6

[SZEDER Gábor]

Hi,

On Tue, Jun 24, 2008 at 05:24:49PM -0400, Nicolas Pitre wrote:
> The problem is probably fixed with this:
> 
> Could you confirm it?
It seems to fix the problem at me.

Gábor