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

Harden read buffer pool #643

Open
vlovich opened this issue Mar 13, 2024 · 2 comments
Open

Harden read buffer pool #643

vlovich opened this issue Mar 13, 2024 · 2 comments

Comments

@vlovich
Copy link
Contributor

vlovich commented Mar 13, 2024

Had a problem where I was pointing to the contents of a ReadResult that I'd dropped (yes, unsafe was involved). Running under ASAN didn't do anything because the underlying memory is still allocated within Glommio.

  1. When building in debug, fill the dropped buffer with a sentinel so that it's more obvious that it's been pointed to (in release could maybe fill just the leading 4 bytes with 0xdeadbeef instead of the full memory region, or control this behavior more fine-grained with feature gates)
  2. Annotate __asan_poison_memory_region when the buffer is returned to the pool when building with ASAN to catch these issues.
  3. Investigate if we put a dropped buffer to the head or tail of the pool (most recently returned buffer should be returned last from the pool to make immediate reuse less likely & thus more likely to trigger ASAN)

No immediate plans to implement this but soliciting feedback on these ideas. @glommer thoughts?

@glommer
Copy link
Collaborator

glommer commented Mar 14, 2024

I don't like 3 very much. Memory issues and "likely" are an explosive combination. Both 1 and 2 seem like good options.
If I understand your plan correctly, 2 seems like the best, as it would work in a release binary as well.

@vlovich
Copy link
Contributor Author

vlovich commented Mar 15, 2024

It's more about a defense in depth strategy since fundamentally there's no way to completely bullet proof catching memory issues due to unsafe & it gets worse when there's a small buffer pool being reused.

Can you help me understand the concerns with option 3? I believe it's a common technique for hardened allocators and I can't imagine there's too many issues: https://llvm.org/docs/ScudoHardenedAllocator.html. Not quite the full Quarantine option but it's ostensibly roughly the same idea, just "0-cost".

Another hardening technique that popped into my head that's I think my own invention is to mmap the buffer pool into many virtual buffer pools. I believe ASAN works on the virtual address, which means then your allocator can ping-pong between different pools to delay reusing the same address almost arbitrarily further. There's some performance cost obviously from the TLB pollution, but probably negligible in practice & not something that would be on unless explicitly opted into / building with ASAN. Not saying I'd ever do it, but fun nerd snipe to rumminate on.

And yes, anything expensive or potentially expensive should be behind a default-off feature flag (e.g. harden-memory-allocator) that is only on if you explicitly turn it on or build with ASAN.

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