Open mfrw opened 6 years ago
/cc @aclements
git bisect points to 2b415549b813ba36caafa34fc34d72e47ee8335c.
@mfrw could you provide information about your hardware, such as the memory size and if you use any swap? None of the builders seem to have seen this panic, so maybe it's got something to do with the machine.
Also, is it just a flake, or does it happen every single time you run go test
?
@mvdan My ram is 8 Gb and yes I do use swap (8 Gb).
No, I think its something weird, it passes all.bash
but when I go inside the encode/gob
package and test, it breaks.
NOTE: It did not break on go1.10.
Btw I tried to test it on my mac and it still broke.
=== RUN TestFuzzRegressions
--- PASS: TestFuzzRegressions (0.00s)
codec_test.go:1444: disabled; run with -gob.fuzz to enable
=== RUN TestFuzzOneByte
signal: killed
FAIL encoding/gob 65.392s
Actually, I can reproduce it too on my linux laptop with 8GB of memory and no swap.
The problem seems to be that the decoder ends up calling reflect.MakeMapWithSize
with a size of 14754407682
. Which can explain why the runtime then fails to allocate this much memory.
But what I don't understand is why @aclements' change to the runtime makes it suddenly fail. I also don't understand what happened earlier with such a large map creation.
Somewhat related: https://github.com/golang/go/issues/20221
I'll send a CL now skipping this index, pending a proper fix in either runtime or gob.
Change https://golang.org/cl/99655 mentions this issue: encoding/gob: work around TestFuzzOneByte panic
The test is passing on my machine now, with the test case being skipped. @aclements, would you like to keep this issue open to investigate if there's anything to fix in the runtime? If not, we can simply keep @josharian's previous issue to decide whether or not the gob package should handle malicious input gracefully.
To summarize, the open question is whether the runtime should ignore the map size hint if there's not enough memory to allocate it:
$ cat map.go
package main
func main() {
m := make(map[string]string, 14754407682)
println(len(m))
}
$ go run map.go
fatal error: runtime: out of memory
runtime stack:
runtime.throw(0x46db4e, 0x16)
/home/bradfitz/go/src/runtime/panic.go:589 +0x6a
runtime.sysMap(0xc004000000, 0x12100000000, 0x4d6bb8)
/home/bradfitz/go/src/runtime/mem_linux.go:156 +0xc7
runtime.(*mheap).sysAlloc(0x4be660, 0x12100000000, 0x0, 0x7fffca2fd298)
/home/bradfitz/go/src/runtime/malloc.go:611 +0x1c7
runtime.(*mheap).grow(0x4be660, 0x9080000, 0x0)
/home/bradfitz/go/src/runtime/mheap.go:920 +0x42
runtime.(*mheap).allocSpanLocked(0x4be660, 0x9080000, 0x4d6bc8, 0x20300000000000)
/home/bradfitz/go/src/runtime/mheap.go:848 +0x337
runtime.(*mheap).alloc_m(0x4be660, 0x9080000, 0x410100, 0x7fa22a673a00)
/home/bradfitz/go/src/runtime/mheap.go:692 +0x119
runtime.(*mheap).alloc.func1()
/home/bradfitz/go/src/runtime/mheap.go:759 +0x4c
runtime.(*mheap).alloc(0x4be660, 0x9080000, 0x7fffca010100, 0x40fef5)
/home/bradfitz/go/src/runtime/mheap.go:758 +0x8a
runtime.largeAlloc(0x12100000000, 0x440001, 0x7fa22c880000)
/home/bradfitz/go/src/runtime/malloc.go:1011 +0x97
runtime.mallocgc.func1()
/home/bradfitz/go/src/runtime/malloc.go:906 +0x46
runtime.systemstack(0x446549)
/home/bradfitz/go/src/runtime/asm_amd64.s:351 +0x66
runtime.mstart()
/home/bradfitz/go/src/runtime/proc.go:1223
goroutine 1 [running]:
runtime.systemstack_switch()
/home/bradfitz/go/src/runtime/asm_amd64.s:311 fp=0xc0000345e0 sp=0xc0000345d8 pc=0x446640
runtime.mallocgc(0x12100000000, 0x463400, 0x444001, 0x7fa22c880000)
/home/bradfitz/go/src/runtime/malloc.go:905 +0x896 fp=0xc000034680 sp=0xc0000345e0 pc=0x409f06
runtime.newarray(0x463400, 0x110000000, 0xf)
/home/bradfitz/go/src/runtime/malloc.go:1040 +0x6a fp=0xc0000346b0 sp=0xc000034680 pc=0x40a27a
runtime.makeBucketArray(0x45c5e0, 0x20, 0x0, 0x0, 0x4ba3a0)
/home/bradfitz/go/src/runtime/map.go:355 +0x184 fp=0xc0000346e8 sp=0xc0000346b0 pc=0x40b064
runtime.makemap(0x45c5e0, 0x36f6e6502, 0xc000034758, 0xc000074058)
/home/bradfitz/go/src/runtime/map.go:321 +0xec fp=0xc000034730 sp=0xc0000346e8 pc=0x40adcc
main.main()
/home/bradfitz/map.go:4 +0x5c fp=0xc000034798 sp=0xc000034730 pc=0x44e55c
runtime.main()
/home/bradfitz/go/src/runtime/proc.go:201 +0x207 fp=0xc0000347e0 sp=0xc000034798 pc=0x4237e7
runtime.goexit()
/home/bradfitz/go/src/runtime/asm_amd64.s:1333 +0x1 fp=0xc0000347e8 sp=0xc0000347e0 pc=0x448591
exit status 2
@aclements, @randall77, is this cool?
We've agreed in the past that the hint passed to make
for a map is only a hint. If we can't allocate enough memory to satisfy the hint, we should ignore it. See #19926. So this is a bug.
If its decided to make map buckets not allocate due to insufficient ram it would be nice to allow this check to be not enforced for small maps. e.g. up to 64 elements and for stack allocated buckets for performance (small local maps) and implementation complexity. Also if maps are small and allocation would fail its more likely another allocation might fail soon which cant be avoided.
@ianlancetaylor I don't think that's what we decided. Reading over the comments on CL 40854, we only prevent panics for obviously bad hints (negative hints, or those which would require allocations larger than the max heap).
We discussed this today. We decided that we should never panic() because of a bad hint, but out of memory is ok. As a practical matter, that means 1) if hint < 0 { hint = 0 } 2) if computing the malloc size overflows an int64, or _MaxMem, set hint = 0. (take newarray's logic and copy it into hinit, replacing panic with hint=0) 3) proceed as before Those who are really passing adversarial data as a hint need to sanitize it first. It seems hard to sanitize the hint inside the map implementation in the general case.
@randall77 Huh, I guess I misremember. So in that case, this is a bug in encoding/gob?
I understand that it is hard to handle a very large hint in the runtime, but it's even harder to handle in packages outside the runtime. I see that I tried to argue that in the CL also. If I was in the discussion you mention, I've forgotten about it.
Please answer these questions before submitting your issue. Thanks!
What did you do?
go test in encoding/gob
What did you expect to see?
Passed tests
What did you see instead?
test_otpt.txt
Does this issue reproduce with the latest release (go1.10)?
No
System details