search

microsoft / mimalloc

mimalloc is a compact general purpose allocator with excellent performance.
MIT License
9.74k stars 791 forks source link

Issue #876: Fix integer overflow on slice_count #877

Closed vstinner closed 2 months ago

vstinner commented 2 months ago

If the slice count doesn't fit into uint32_t, consider that the memory allocation failed.

On Linux s390x, allocating around 8,589,934,592 GiB with mmap() works thanks to overcommit on a machine with 8 GiB of memory:

mmap(NULL,
     0x8000000000400000,
     PROT_READ | PROT_WRITE,
     MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE,
     -1, 0)
vstinner commented 2 months ago

microsoft.mimalloc Failing after 3m — Build #20240415.2 failed

Hum, it doesn't seem to be related to my change:

1: =================================================================
1: ==1864==ERROR: LeakSanitizer: detected memory leaks
1: 
1: Direct leak of 4097 byte(s) in 1 object(s) allocated from:
1:     #0 0x56446f10728e in __interceptor_malloc (/home/vsts/work/1/s/debug-asan-clang/mimalloc-test-api+0xa228e) (BuildId: f63f94b5f9da6b5b7cc6c667411035f8024a4297)
1:     #1 0x56446f0bb0f6 in __interceptor_realpath (/home/vsts/work/1/s/debug-asan-clang/mimalloc-test-api+0x560f6) (BuildId: f63f94b5f9da6b5b7cc6c667411035f8024a4297)
1:     #2 0x7f8fd7c0f4c1 in mi_heap_realpath /home/vsts/work/1/s/src/alloc.c:396:19
1:     #3 0x7f8fd7c0f544 in mi_realpath /home/vsts/work/1/s/src/alloc.c:420:10
1:     #4 0x56446f1458f9 in main /home/vsts/work/1/s/test/test-api.c:299:15
1:     #5 0x7f8fd7829d8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) (BuildId: c289da5071a3399de893d2af81d6a30c62646e1e)
1: 
1: SUMMARY: AddressSanitizer: 4097 byte(s) leaked in 1 allocation(s).
daanx commented 2 months ago

Thanks so much for spotting this! I fixed it differently by defining a MI_MAX_ALLOC_SIZE and limiting it to at most MI_SEGMENT_SLICE_SIZE * (UINT32_MAX-1) (and adding assertions). If you can, can you verify this works for your tests?

vstinner commented 2 months ago

I fixed it differently by defining a MI_MAX_ALLOC_SIZE and limiting it to at most MI_SEGMENT_SLICE_SIZE * (UINT32_MAX-1) (and adding assertions).

Do you have a link to the fix? https://github.com/microsoft/mimalloc/commit/5050b630389bb4d2013d1e82cc1b2c2d9b65b963 only defines MI_MAX_ALLOC_SIZE as PTRDIFF_MAX. Is it the same fix?

daanx commented 2 months ago

Ah, for dev PTRDIFF_MAX is still the limit as it has no "slices". (dev = v1.x), but for the dev-slice branch the new limit is UINT32_MAX as in the commit 78418b3. (dev-slice == v2.x).

vstinner commented 2 months ago

If you can, can you verify this works for your tests?

Sorry, but it's quite complicated to set up the test :-/ Thanks for the fix anyway!