panic: index out of range (sonar)

[ssoroka]

panic: runtime error: index out of range

goroutine 11 [running]:
panic(0x3828a0, 0xc4200120f0)
    /usr/local/go/src/runtime/panic.go:500 +0x1a1
main.(*Slave).parseSonarData(0xc4200ee700, 0xc42256c6fc, 0xb78ff, 0xb7904, 0xc4202df838, 0x71090, 0xc4202df710)
    github.com/dvyukov/go-fuzz/go-fuzz/sonar.go:79 +0x723
main.(*Slave).processSonarData(0xc4200ee700, 0xc42133a000, 0x11249, 0x11249, 0x2700000, 0xffffb, 0x100000, 0x1, 0x11201)
    github.com/dvyukov/go-fuzz/go-fuzz/sonar.go:88 +0x169
main.(*Slave).smash(0xc4200ee700, 0xc42133a000, 0x11249, 0x11249, 0x1)
    github.com/dvyukov/go-fuzz/go-fuzz/slave.go:415 +0x12ab
main.(*Slave).triageInput(0xc4200ee700, 0xc42133a000, 0x11249, 0x11249, 0x1, 0x1, 0x1)
    github.com/dvyukov/go-fuzz/go-fuzz/slave.go:289 +0x362
main.(*Slave).loop(0xc4200ee700)
    github.com/dvyukov/go-fuzz/go-fuzz/slave.go:154 +0x3ed
created by main.slaveMain
    github.com/dvyukov/go-fuzz/go-fuzz/slave.go:130 +0xc64

Seems the offending line is: res = append(res, SonarSample{&ro.sonarSites[id], flags, [2][]byte{v1, v2}})

[dvyukov]

Is it possible that the test program has some background goroutines?

[ssoroka]

I don't think so. It's for image manipulation. I'll double check

[dvyukov]

Full panic message should contain all goroutines.

[ssoroka]

Ok, I checked into this and I am using github.com/nfnt/resize and github.com/disintegration/imaging , which both use goroutines.
The above stack trace is the only one I get on panic, though. Maybe the defer recover() in main.go is losing it?

Could try something like:

            runtime.Stack(stackBuff, true)
            log.Fatal("Panicked:", x, string(stackBuff))

[dvyukov]

Then I think the problem is in go-fuzz-dep/sonar.go:

// Sonar is called by instrumentation code to notify go-fuzz about comparisons.
// Low 8 bits of id are flags, the rest is unique id of a comparison.
func Sonar(v1, v2 interface{}, id uint32) {
...
        pos := atomic.LoadUint32(&sonarPos)
        for {
                if pos+n > uint32(len(sonarRegion)) {
                        return
                }
                if atomic.CompareAndSwapUint32(&sonarPos, pos, pos+n) {
                        break
                }
                pos = atomic.LoadUint32(&sonarPos)
        }
        copy(sonarRegion[pos:pos+n], buf[:])
}

While the increment is atomic, go-fuzz can see partially written data from background goroutines in sonarRegion,

[ssoroka]

So if we add a mutex over the whole thing it should be okay. I can test that out.

[dvyukov]

Unfortunately it's not that simple. sonarRegion is shared memory region. sync.Mutex does not work across processes.
I would also be concerned about performance of mutex approach.

[dvyukov]

Ideally we have some shared memory protocol that allows to understand what slots are fully-written and what are not.

[ssoroka]

I tried adding a mutex.Lock() defer mutex.Unlock() just before that block. It seemed to help, but it did eventually fail with the error from #146

[dvyukov]

Are you also overriding runtime.GOMAXPROCS? go-fuzz sets GOMAXPROCS to 1. And I can't reproduce the crashes without overriding GOMAXPROCS.

Both background goroutines and overriding GOMAXPROCS is bad for fuzzing? Can you remove at least overriding of GOMAXPROCS? It should fix crashes.

[dvyukov]

I've submitted a reproducer for the bug. But it does not reproduce by default, only if runtime.GOMAXPROCS call is uncommented. I don't want to uncomment it, because it's not the recommended way of running go-fuzz.

[ssoroka]

I'm not explicitly overriding GOMAXPROCS. I'll review all the code I'm importing to confirm.
I don't think it's good enough to say "go-fuzz shouldn't use multiple processs", because the library it's testing uses multiple processors. You're eliminating a class of crashers that go-fuzz will not be able to find without multiple processors.

[dvyukov]

The race detector can find these bugs even with GOMAXPROCS=1.
Also they are usually not considerably affected by the function input. I mean if something starts a goroutine, then it starts it in either case.

[ssoroka]

FYI, I'm not setting GOMAXPROCS anywhere. I could maybe build a test case that exhibits the bug.

re race detection, I don't see where go-fuzz turns on race detection?

[dvyukov]

I could maybe build a test case that exhibits the bug.

That would be useful.

re race detection, I don't see where go-fuzz turns on race detection?

It doesn't. But if we want to catch these bugs with go-fuzz, it would be the right approach.

[dgryski]

Even without a fix yet, it seems like we can add a note to the documentation about using go-fuzz with goroutines.

[catenacyber]

What is the status on this ? (using go-fuzz with goroutines)
I have the case for gonids project

[dvyukov]

I don't think anything has changed here since the last message. The Open status of the issue is still valid.

[catenacyber]

Thanks.
For information, I think the problem with gonids got triggered because one goroutine had too many levels of recursion (the stack may have overflown somewhere bad...)