Skip to content
This repository was archived by the owner on Sep 11, 2020. It is now read-only.

SIGSEGV in fillREFDeltaObjectContent #568

Closed
vmarkovtsev opened this issue Aug 28, 2017 · 8 comments · Fixed by #646
Closed

SIGSEGV in fillREFDeltaObjectContent #568

vmarkovtsev opened this issue Aug 28, 2017 · 8 comments · Fixed by #646

Comments

@vmarkovtsev
Copy link
Contributor

I use the following code go run bug.go:

package main

import (
"io"
"gopkg.in/src-d/go-git.v4"
)

func main() {
	testRepository, err := git.PlainOpen("/Users/sourced/Projects/hercules")
	if err == nil {
		iter, err := testRepository.CommitObjects()
		if err == nil {
			commits := -1
			for ; err != io.EOF; _, err = iter.Next() {
				if err != nil {
					panic(err)
				}
				commits++
				if commits >= 100 {
					return
				}
			}
		}
	}
}

/Users/sourced/Projects/hercules points to the decompressed archive I've attached: hercules-bug.tar.gz

I get:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x48 pc=0x1164740]

goroutine 1 [running]:
gopkg.in/src-d/go-git.v4/plumbing/format/packfile.(*Decoder).fillREFDeltaObjectContent(0xc4201584e0, 0x15539c0, 0xc42011e240, 0xbb3572d01cac4e99, 0x26845ca747e51598, 0xf500ea5d, 0x1008e9c, 0xc420049d94, 0x24cd747d)
	/tmp/ggbug/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/decoder.go:364 +0xf0
gopkg.in/src-d/go-git.v4/plumbing/format/packfile.(*Decoder).decodeByHeader(0xc4201584e0, 0xc42011e200, 0x154bb40, 0xc4200102c0, 0xc420118201, 0x14)
	/tmp/ggbug/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/decoder.go:288 +0x104
gopkg.in/src-d/go-git.v4/plumbing/format/packfile.(*Decoder).decodeIfSpecificType(0xc4201584e0, 0xc42011e200, 0x0, 0x0, 0x0, 0x0)
	/tmp/ggbug/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/decoder.go:234 +0x306
gopkg.in/src-d/go-git.v4/plumbing/format/packfile.(*Decoder).DecodeObject(0xc4201584e0, 0x0, 0x0, 0x0, 0x0)
	/tmp/ggbug/src/gopkg.in/src-d/go-git.v4/plumbing/format/packfile/decoder.go:219 +0xa7
gopkg.in/src-d/go-git.v4/storage/filesystem.(*packfileIter).Next(0xc420141770, 0xc420049ed0, 0x110a1a1, 0xc4200cdce0, 0x15539c0)
	/tmp/ggbug/src/gopkg.in/src-d/go-git.v4/storage/filesystem/object.go:412 +0x41
gopkg.in/src-d/go-git.v4/plumbing/storer.(*MultiEncodedObjectIter).Next(0xc420179260, 0xc42006e480, 0x15539c0, 0xc42049fa00, 0xc4200cdce0)
	/tmp/ggbug/src/gopkg.in/src-d/go-git.v4/plumbing/storer/object.go:205 +0x47
gopkg.in/src-d/go-git.v4/plumbing/object.(*storerCommitIter).Next(0xc420179280, 0xc4200cdce0, 0x0, 0x0)
	/tmp/ggbug/src/gopkg.in/src-d/go-git.v4/plumbing/object/commit.go:271 +0x37
main.main()
	/tmp/ggbug/bug.go:14 +0xbf
exit status 2

If I run git gc inside the repository, everything works properly and there are no segfaults.

@mcuadros
Copy link
Contributor

can you send us the commit of go-git are you using?

@vmarkovtsev
Copy link
Contributor Author

It's v4 release cloned yesterday

@mcuadros
Copy link
Contributor

Please the commit, the v4 was updated yesterday

@vmarkovtsev
Copy link
Contributor Author

I can reproduce it with the latest release 7aa9d15

@mcuadros mcuadros added the bug label Sep 5, 2017
@jsperayil
Copy link

I'm facing the same issue.

It is caused by d.DecodeObjectAt returning (nil, nil) in this case.

@vmarkovtsev
Copy link
Contributor Author

@mcuadros I am not the only one who runs go-git on "wrong" data now ;-)

@jsperayil
Copy link

I did a bit more digging and found the place where nil, nil is returned in this case.

And another place where the condition obj == nil is handled when the previous code point returns nil,nil.

I don't have a clear understanding of the internals of go-git.

My guess is that the fix is just replacing

return d.DecodeObjectAt(int64(e.Offset))

with

obj, err := d.DecodeObjectAt(int64(e.Offset))
if obj == nil && err == nil {
    err = plumbing.ErrObjectNotFound
}
return obj, err

over here.

Can somebody with a better understanding confirm that ?

@mcuadros
Copy link
Contributor

mcuadros commented Nov 9, 2017

The problem is that this should not happen, if this object not exists, then is a malformed packfile. If we just return an error, then instead of a panic you will get a error, but the read of the packfile will be unsuccessful anyway.

So the way to solve it, is see what's is happening with this packfile and if this packfile is supported by cgit, if it is, we should supported to.

Based on what we know, a delta can't reference a object that hasn't been read previously.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants