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

Another unexpected behavior in markus allocator #3

Closed
insuyun opened this issue Feb 6, 2021 · 2 comments
Closed

Another unexpected behavior in markus allocator #3

insuyun opened this issue Feb 6, 2021 · 2 comments

Comments

@insuyun
Copy link

insuyun commented Feb 6, 2021

Hi. Similar to #1, we found that markus still allows reclamation.

#include <assert.h>
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <malloc.h>


void* p[256];
uintptr_t buf[256];

int main() {
  p[0] = malloc(-1);
  p[1] = malloc(926756);
  free(p[1]);
  p[2] = malloc(350500);

  assert(p[1] <= p[2] && p[2] < p[1] + 926756);
  fprintf(stderr, "reclaimed!\n");

  // cause segmentation fault
  memset(p[2], 0x42, 926756);
}
$ ./poc
GC Warning: Failed to expand heap by -4096 bytes
GC Warning: Out of Memory! Heap size: 0 MiB. Returning NULL!
reclaimed!
[2]    4809 segmentation fault  LD_PRELOAD= ./markus-reclaim

Moreover, the reclaimed memory is not fully accessible,
which violates the convention of an allocator, I believe.

@SamAinsworth
Copy link
Owner

Thanks Insu!

This was a curious one. The oversized alloc to uint(-1) causes the allocator to fail on sbrk. In the default Boehm Allocator setup, this then causes it to both a) try mmap, and b) lock in to mmap for all future allocations, regardless of whether mmap worked.

Part b) is what causes this bug. For some reason (I'm not sure if this is a MarkUs issue or a BoehmGC issue) the allocations in that new Mmap space (rather than the contiguous region of Sbrked space, which itself can be mmaped and munmapped freely, and still be checked) will free themselves even with a dangling pointer.

Since this switch over to Mmap isn't particularly intended behaviour (and isn't used in the current version without an attempt at an oversized allocation), I've simply disabled that feature.

In the fixed version, the allocation of p[2] now uses different memory from p[1], as the virtual address space it resides in is still unavailable due to the dangling pointer.

As for the memset, this is intended functionality: you've gone past the bounds of p[2]'s 350500-sized allocation, which is a spatial safety bug (and more directly, undefined behaviour, which is why MarkUs is able to throw a segmentation fault). The MarkUs allocator is still allowed to reuse that space if it wishes -- but since it correctly believes nobody else should have access to it at present, it leaves the virtual address space munmapped.

Cheers,
Sam

@insuyun
Copy link
Author

insuyun commented Feb 6, 2021

Thanks for your quick fix!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants