[compiler-rt] [scudo] Add primary option to zero block on dealloc. (PR #142394)

[via llvm-commits]

ChiaHungDuan wrote:

> Thanks for taking the time to reply.
> 
> > Zeroing-on-free is also a security feature ans that's why I think it is good to have. But it seems to me this is introduced for specific platform, then we may want to consider if we can implement it on platform side.
> 
> I made some conclusive measurements on Fuchsia. I don't have measures on Android but I this is will be positive too, given that linux zram store efficiently pages with a unique value.

If this is supposed to be generally supported, then it shouldn't be only for primary allocator. Like the zero-on-allocation, it's used for all the blocks. Unless we have a good reason and it's for general cases (not for some specific cases). OTOH, we have the page releasing mechanism which also helps with memory usage. You can do a mandatory clean up as well. One thing I'm not sure is the cost of having the kernel detects the zero page, it seems not a cheap operation like does it check the page status periodically?
> 
> > For example, the malloc/free hooks.
> 
> I see a few challenges because `__scudo_deallocate_hook`:
> 
> * runs outside of the allocator lock, hence a given block could be in use when the hook is called.
> * does not know about the allocation header which also needs to be cleared.
> * runs indistinctly on primary and secondary allocator, wheras this change is a primary allocator optimization.
> * `__scudo_deallocate_hook` does not know the object size (easily fixed)
> 

Sorry, I forgot you also want to clean the header. Then this is another behavior which is specific to a certain case. Adding zeroing-on-free, I may expect the zeroing cleans everything except the header (because it helps something like double-free-detection)

> > In addition, the size of blocks in the primary allocator can be large (like several pages), it depends on the configuration. So even if you only zeroing blocks in primary allocator, it can still cause some performance issue.
> 
> Indeed. That should be in the same ballpark as zeroing on allocation. I thought about relaying on a platform specific function later on that would use kernel operation to zero the memory. That feature could be implemented in memset or scudo. I profiled scudo allocation and deallocation on my system and the effect is small. I observe not effect on macro bencharks.
> 

Not only the micro-benchmark, we should also take real cases into consideration. When turning on zeroing-on-alloc, we see significant performance impact on Android (especially for some processes like camera which is sensitive to fps) Thus I'm not sure what you mean "the effect is small". Even for microbenchmark, I believe you still see performance regression on free because it just introduce additional operations. I would like to see if we have more details about the measurements. 

> On the positive side, this decommits 30 MiB of memory on our system which is significant, I observe a reduction of the number of pages moved to zram, and a decrease in the time spent by the kernel on compression and decompression.
> 
> > We do have a way to implement this only for certain sizes but I would suggest having more discussion before we make it a formal feature in Scudo
> 
> Sure, how can I get the discussion rolling?
We can do it here. The main concern for me is that I'm not persuaded by the reason why we only support zeroing memory on free for primary and clean everything in the block. Scudo is an userspace memory allocator, if there's any performance/memory optimization relying on the OS/Kernel, then we should be careful about the expectation of new features. For example, it's better not to say the OS will release the zero pages. And the users are not only Fuchsia/Android, there are still few different platforms (yes, very few of them, but it's not only for one or two platforms). It's better to make changes based on more general use cases.

https://github.com/llvm/llvm-project/pull/142394