Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v0.9.0 panics on OpenBSD #8211

Closed
qbit opened this issue Jun 23, 2021 · 5 comments · Fixed by multiformats/go-multiaddr#155 or #8215
Closed

v0.9.0 panics on OpenBSD #8211

qbit opened this issue Jun 23, 2021 · 5 comments · Fixed by multiformats/go-multiaddr#155 or #8215
Labels
kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization

Comments

@qbit
Copy link

qbit commented Jun 23, 2021

Version information:

go-ipfs version: 0.9.0
Repo version: 11
System version: amd64/openbsd
Golang version: go1.16.5

Description:

After running for a period of time, ipfs daemon panics with the following:

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

goroutine 270 [running]:
github.com/multiformats/go-multiaddr/net.(*maListener).Accept(0xc0006b49a0, 0xc00287de18, 0xb23a5d, 0xc000019980, 0xc008136a40)
        /build/pobj/go-ipfs-0.9.0/go/pkg/mod/github.com/multiformats/go-multiaddr@v0.3.2/net/net.go:259 +0x112
github.com/libp2p/go-tcp-transport.(*tcpListener).Accept(0xc0007af950, 0xc00287de70, 0xc00287de78, 0x20, 0xc0007d8a80)
        /build/pobj/go-ipfs-0.9.0/go/pkg/mod/github.com/libp2p/go-tcp-transport@v0.2.2/tcp.go:66 +0x37
github.com/libp2p/go-tcp-transport.(*tracingListener).Accept(0xc0001636f0, 0x0, 0x0, 0xc001438fc0, 0x1adbf60)
        /build/pobj/go-ipfs-0.9.0/go/pkg/mod/github.com/libp2p/go-tcp-transport@v0.2.2/metrics.go:226 +0x37
github.com/libp2p/go-libp2p-transport-upgrader.(*listener).handleIncoming(0xc001438fc0)
        /build/pobj/go-ipfs-0.9.0/go/pkg/mod/github.com/libp2p/go-libp2p-transport-upgrader@v0.4.2/listener.go:77 +0xf7
created by github.com/libp2p/go-libp2p-transport-upgrader.(*Upgrader).UpgradeListener
        /build/pobj/go-ipfs-0.9.0/go/pkg/mod/github.com/libp2p/go-libp2p-transport-upgrader@v0.4.2/upgrader.go:49 +0x18d

The daemon also produces a plethora of this error while running:

2021-06-23T06:01:16.574-0600    ERROR   tcp-tpt go-tcp-transport@v0.2.2/tcp.go:45       Failed set keepalive period: set tcp4 REDACTED:1777->REDACTED:30504: protocol not available
@qbit qbit added kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization labels Jun 23, 2021
@qbit
Copy link
Author

qbit commented Jun 23, 2021

The error from the keepalive bit seems to be from https://github.com/golang/go/blob/2ebe77a2fda1ee9ff6fd9a3e08933ad1ebaea039/src/net/tcpsockopt_openbsd.go#L13

So that might not be the issue.

@marten-seemann
Copy link
Member

Agreed, this is definitely not related to the socket option.

The problem is in https://github.com/multiformats/go-multiaddr/blob/2f4fae4104af2df7036afcdd93d2075845e68ed0/net/net.go#L234-L267.
It looks like the net.Listener returns a net.Conn where the LocalAddr() is nil. It looks like the problem was introduced in multiformats/go-multiaddr#135, which was merged after the v0.3.1 release, so it makes sense that this only occurs now.

Now I'm not sure why a net.Conn would return nil in LocalAddr(), I would have expected it to return a non-nil value in every case. The easy fix is to guard against unexpected nil values, if we can't come up with a reasonable explanation here.

@qbit
Copy link
Author

qbit commented Jun 23, 2021

I figure this is more of a question for this thread:

Given libp2p/go-tcp-transport#80 (comment)

I guess OpenBSD users then have to live with a slightly suboptimal behavior then

It looks like go-tcp-transport just sets the keepalive to 30 seconds. I maintain the port of go-ipfs on OpenBSD and we have a doc that makes a number of tuning recommendations - Would recommending that users set their keepalive to 30 seconds be worth mentioning?

@marten-seemann
Copy link
Member

We probably should keep this open until the change is bubbled up here.

@marten-seemann
Copy link
Member

marten-seemann commented Jun 24, 2021

@qbit depending on the default value that OpenBSD uses, it might or might not be worth it.

@aschmahmann aschmahmann mentioned this issue Jul 16, 2021
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws) need/triage Needs initial labeling and prioritization
Projects
None yet
2 participants