Layering snmalloc on top of another allocator

[mjp41]

There are cases where it would be useful for snmalloc to pass certain requests to a different allocator.

1. When overriding an allocator, being able to decide if an allocation is not from snmalloc and pass it to the original allocator.
2. When integrating with systems such as GWP-Asan, here memory comes from a different source and structure, and wants to be called some of the time.

The documentation for GWP-Asan has

#ifdef INSTALL_GWP_ASAN_STUBS
  gwp_asan::GuardedPoolAllocator GWPASanAllocator;
#endif

void* YourAllocator::malloc(..) {
#ifdef INSTALL_GWP_ASAN_STUBS
  if (GWPASanAllocator.shouldSample(..))
    return GWPASanAllocator.allocate(..);
#endif

  // ... the rest of your allocator code here.
}

This I think should involve a concept of the shape:

class SecondaryAllocator
{
   void* alloc(size_t);
   void dealloc(void*);
};

We would put calls on the slow path for allocation

• For small allocations here:

  snmalloc/src/snmalloc/mem/corealloc.h

  Line 881 in 33b7f65
• For large allocations here:

  snmalloc/src/snmalloc/mem/localalloc.h

  Line 186 in 33b7f65

This would involve adding something like

void* result = SecondaryAllocator::alloc(size);
if (result != nullptr)
  return result;

For deallocation there would just be a change here:

• snmalloc/src/snmalloc/mem/localalloc.h

  Lines 698 to 704 in 33b7f65

  	// If p_tame is not null, then dealloc has been call on something
  	// it shouldn't be called on.
  	// TODO: Should this be tested even in the !CHECK_CLIENT case?
  	snmalloc_check_client(
  	mitigations(sanity_checks),
  	p_tame == nullptr,
  	"Not allocated by snmalloc.");

This would involve adding code like

SecondaryAllocator::free(p_tame);

We could then define a Default class as

class DefaultSecondaryAllocator
{
  SNMALLOC_FAST_PATH static void* alloc(size_t)
  {
    return nullptr;
  }

  SNMALLOC_FAST_PATH static void free(void* p)
  {
    // If p_tame is not null, then dealloc has been call on something 
    // it shouldn't be called on. 
    // TODO: Should this be tested even in the !CHECK_CLIENT case? 
    snmalloc_check_client( 
      mitigations(sanity_checks), 
      p_tame == nullptr, 
      "Not allocated by snmalloc."); 
  }
};

and for GWP something line

class GWPSecondaryAllocator
{
  SNMALLOC_FAST_PATH static void* alloc(size_t size)
  {
    if (SNMALLOC_UNLIKELY((GWPASanAllocator.shouldSample())
    {
       return GWPASanAllocator.allocate(size);
    }
    return nullptr;
  }

  SNMALLOC_FAST_PATH static void free(void* p)
  {
    GWPAsanAllocator.deallocate(p);
  }
};

Providing a SecondaryAllocator as a compile time option would allow us to integrate various systems cleanly.

Unknowns

• How to actually integrate with GWPAsan builds
• If this design works for more than just GWPAsan
• Precise way to integrate the configuration of the SecondaryAllocator into the build. Perhaps part of class CommonConfig, and override by specific configurations.

[mjp41]

@nwf @davidchisnall @SchrodingerZhu any thoughts on this? Seems reasonably low effort to potential reward.

[SchrodingerZhu]

First, this does seem like a good idea.
I am not sure about GWPASan but maybe you can try scudo first. It is a purely standalone allocator without any dependency to c/c++ runtime. Hence it may be easy to integrate. Scudo itself also has a GWPASan flag but I did not try that before.

[SchrodingerZhu]

Actually, I checked the Scudo interface. API-wise I think GWPASan are easier to work with. Let me check more about how scudo is integrating GWPASan and I can report back.

[SchrodingerZhu]

See also https://github.com/llvm/llvm-project/blob/90a776fbdb35ef6ac6f28a19937e88abd2e4611b/compiler-rt/lib/scudo/standalone/combined.h#L207

[mjp41]

Thanks for investigating. Reading that file, we also need to do some integration around realloc/malloc_usable_size. I'm not sure if we can find the start of an object easily with GWP-Asan to get external_pointer working as well.

@SchrodingerZhu I won't have time to work on this for a while, so if you're interested you are welcome to have a go. Otherwise I'll try to find some time, but it might be a month or two.

[nwf]

I see no real harm in the proposed refactoring to allow for compile-time pluggable behaviors here. Just to state the obvious: the branch on the malloc side should optimize away given the Default and we already have a branch on the free side. (Though some part of me does wonder if there's a way to composite snmalloc and more than one other allocator... but that probably requires that everyone involved understand the pagemap at least a little bit, and that's probably a bridge too far.)

One nit-picky, but just to say... in the example above, the DefaultSecondaryAllocator::free() does not have access to the would-be-domesticated p_tame. However it'd only be called if p_tame == null, so maybe its body is snmalloc_check_client(mitigations(sanity_checks), true, "Not allocated by snmalloc")?