quick fix for OOM panic that has been plaguing us #1181
Conversation
whyrusleeping
added
kind/bug
| @@ -130,6 +134,10 @@ func (r *etmReader) Read(buf []byte) (int, error) { | |||
| return 0, err | |||
| } | |||
|
|
|||
| if fullLen > MaxMsgSize { | |||
| return 0, ErrMaxMessageSize | |||
| } | |||
There was a problem hiding this comment.
oh this makes sense. we may want to bump it even lower. what's going on here is that length prefix may receive random input from the other side, interpreting it as a length.
what's weird to me is that this happens after encryption has been setup correctly. it would make more sense if we were seeing this before the handshake finished-- meaning we're speaking to something else entirely.
This problem suggests we should use a well-defined small header as the first message (i.e. like the protocol-stream idea), before reading "length prefixes" from the network. (that, or cap the lengths to a reasonable size always. (e.g. handshake messages should be below 4KB)
|
Yeah, not great but LGTM. We might want to merge the go-msgio quickfix, too. |
|
This LGTM. see my comment above o/ -- the problem is larger. perhaps we should make msgio readers have a tunable max size. the handshake messages shouldn't be very large. @cryptix jbenet/go-msgio#4 is merged. we may want to make it tunable, because most msgio messages will be well below 512MB. (even well below 1MB) |
Yup, but we need update our godep here, I think. I also think that 512mb is too much, just wanted something that doesn't instantly oom every maschine. |
|
@cryptix Think of the Pis! |
|
8MB should be a good upper limit — On Sat, May 2, 2015 at 1:42 PM, Jeromy Johnson notifications@github.com
|
|
@jbenet what do you want done here? |
|
@whyrusleeping happy to merge this but it's a bandaid -- larger fix needed. |
|
@jbenet what larger fix are you thinking about? |
|
The larger fix needed is to (any or maybe all):
|
quick fix for OOM panic that has been plaguing us
I'm pretty sure this is the right way to address this, but it could definitely use a look over and some thought.