Fix panic: runtime error: slice bounds out of range #5264
Conversation
The block count was an uint16 when incrementing the index location which was an int32. This caused the value the uint16 value to overflow before the index location was incremented causing the wrong location to be read on the next iteration of the loop. This triggers the slice out of range errors. Added a test that recreates the panic seen in #5257 and possibly #5202 which is older code. Fixes #5257
| @@ -607,7 +607,7 @@ func (d *indirectIndex) UnmarshalBinary(b []byte) error { | |||
| i++ | |||
|
|
|||
| // 2 byte count of index entries | |||
| count := btou16(b[i : i+indexCountSize]) | |||
| count := int32(btou16(b[i : i+indexCountSize])) | |||
There was a problem hiding this comment.
I must be dense, but I don't see how this affects anything. Where exactly does a 16-bit wide count affect anything?
There was a problem hiding this comment.
I.e. by its nature btou16 returns a 16-bit type. And since count is never incremented, how it could it overflow?
There was a problem hiding this comment.
You can't see where it is incremented in the diff. It happens on here: https://github.com/influxdb/influxdb/pull/5264/files#diff-2226d419783ec10497f3bab10d1c4300R619
See this snippet for essentially what is happening: https://play.golang.org/p/zMYh_UOw4h
There was a problem hiding this comment.
OK, I see it now. 28 is shoved into 16 bits too.
The change is a little obscure. Would it be better to move the casting to nearer where it matters? That would probably make it clearer to the next guy.
I.e.
i += int32(count) - 1 * int32(indexEntrySize))
You get the idea -- deal with the overflow at the point where it matters.
|
+1 |
|
I have some feedback but I understand the fix now. |
|
@jwilder installed your branch on those nodes and confirmed that it fixed the issue. nice job! 👍 |
Fix panic: runtime error: slice bounds out of range
|
@jwilder I was looking at the Changelog and I cannot see this issue (#5264): https://github.com/influxdata/influxdb/blob/master/CHANGELOG.md |
|
@ivanscattergood I updated the change log. It should be in the current nightlies. If you could test it out for those two issues, that would be great. |
The block count was an uint16 when incrementing the index location
which was an int32. This caused the value the uint16 value to overflow
before the index location was incremented causing the wrong location
to be read on the next iteration of the loop. This triggers the slice
out of range errors.
Added a test that recreates the panic seen in #5257 and possibly #5202 which
is older code.
Fixes #5257