evilsocket / opensnitch

OpenSnitch is a GNU/Linux interactive application firewall inspired by Little Snitch.
GNU General Public License v3.0
10.74k stars 498 forks source link

fatal error: concurrent map read and map write #265

Closed gustavo-iniguez-goya closed 3 years ago

gustavo-iniguez-goya commented 5 years ago

It seems that this error occur when denying a connection having the UI opened. It was reported previously here #225 as a comment of another request.

Steps to try to reproduce it:

full log daemon_concurrent_map_error.txt

(...)
jun 17 21:41:15 vv opensnitchd[28438]: github.com/evilsocket/opensnitch/daemon/vendor/google.golang.org/grpc.Invoke(0xa33880, 0xc0016e0f60, 0x9a7414, 0x11, 0x9278a0, 0xc001ce1010, 0x91e840, 0xc001353200, 0xc001362000, 0x0, ...)
jun 17 21:41:15 vv opensnitchd[28438]:         /home/v/go/src/github.com/evilsocket/opensnitch/daemon/vendor/google.golang.org/grpc/call.go:60 +0xc1 fp=0xc001be5e28 sp=0xc001be5da8 pc=0x833fe1
jun 17 21:41:15 vv opensnitchd[28438]: github.com/evilsocket/opensnitch/daemon/ui/protocol.(*uIClient).Ping(0xc001354028, 0xa33880, 0xc0016e0f60, 0xc001ce1010, 0x0, 0x0, 0x0, 0x42e3a1, 0x9c6e20, 0xc001be5f38)
jun 17 21:41:15 vv opensnitchd[28438]:         /home/v/go/src/github.com/evilsocket/opensnitch/daemon/ui/protocol/ui.pb.go:452 +0xd2 fp=0xc001be5eb0 sp=0xc001be5e28 pc=0x856192
jun 17 21:41:15 vv opensnitchd[28438]: github.com/evilsocket/opensnitch/daemon/ui.(*Client).ping(0xc001242780, 0xbf3a1916c82e7018, 0x34f2153aa7, 0xe2e060, 0x0, 0x0)
jun 17 21:41:15 vv opensnitchd[28438]:         /home/v/go/src/github.com/evilsocket/opensnitch/daemon/ui/client.go:128 +0x1e0 fp=0xc001be5f60 sp=0xc001be5eb0 pc=0x861700
jun 17 21:41:15 vv opensnitchd[28438]: github.com/evilsocket/opensnitch/daemon/ui.(*Client).poller(0xc001242780)
jun 17 21:41:15 vv opensnitchd[28438]:         /home/v/go/src/github.com/evilsocket/opensnitch/daemon/ui/client.go:76 +0x18b fp=0xc001be5fd8 sp=0xc001be5f60 pc=0x86113b
jun 17 21:41:15 vv opensnitchd[28438]: runtime.goexit()
jun 17 21:41:15 vv opensnitchd[28438]:         /usr/lib/go-1.11/src/runtime/asm_amd64.s:1333 +0x1 fp=0xc001be5fe0 sp=0xc001be5fd8 pc=0x45cc21
jun 17 21:41:15 vv opensnitchd[28438]: created by github.com/evilsocket/opensnitch/daemon/ui.NewClient
jun 17 21:41:15 vv opensnitchd[28438]:         /home/v/go/src/github.com/evilsocket/opensnitch/daemon/ui/client.go:48 +0xd6
jun 17 21:41:15 vv opensnitchd[28438]: goroutine 1 [chan receive]:
jun 17 21:41:15 vv opensnitchd[28438]: main.main()     
gustavo-iniguez-goya commented 5 years ago

After much much debugging, I think the problem is when passing map[]s by reference in statistics.Serialize() to c.client.Ping(), instead of by value. The address of the maps is read inside c.client.Ping() where it cannot be locked, and the same address can be modified in incMap() at the same time.

We could either make a copy of the maps in every Ping, or lock them by exporting the mutex lock methods:

daemon/statistics/stats.go:

- sync.Mutex
+ sync.RWMutex

func (s *Statistics) R_Lock() {
    s.RLock()
}

func (s *Statistics) R_Unlock() {
    s.RUnlock()
}

daemon/ui/client.go:

    p := protocol.PingRequest{
        Id:    reqId,
        Stats: c.stats.Serialize(),
    }
    c.stats.R_Lock()
    pong, err := c.client.Ping(ctx, &p)
    c.stats.R_Unlock()