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

[Feature request] Heap canary #58

Open
Kazurin-775 opened this issue Apr 12, 2022 · 3 comments
Open

[Feature request] Heap canary #58

Kazurin-775 opened this issue Apr 12, 2022 · 3 comments

Comments

@Kazurin-775
Copy link

Recently I occurred a heap corruption bug in one of my real life projects:

Kernel panic - aborting: at /home/kazurin/.cargo/registry/src/rsproxy.cn-8f6827c7555bfaf8/linked_list_allocator-0.9.1/src/hole.rs:311: attempt to add with overflow

It took me long before I was able to discover the root cause, due to the complication of kernel debugging and the need to read the source code of various crates. It might be handy to have some simple heap canary mechanism (just like what Valgrind does) built into the allocator in such cases.

Features that I consider useful:

  • Ability to detect the corruption of heap control structures (namely the Holes) using some predefined magic bytes in the structure
  • Ability to reveal the corrupted structure's address in the panic message (for easier debugging)
  • Ability to verify the integrity of the whole heap at any time
  • The heap canary feature should be completely optional, in order not to cause preformance impact on production code

If such features are acceptable but not planned, perhaps I could do a PR in the future :)

@phil-opp
Copy link
Member

I like that idea and I would be happy to accept a PR for it!

@jamesmunns
Copy link
Collaborator

As a part of this/alternative, we could also add some "strict checking" in the Cursor traversal logic that ensures that the "next pointer" for each hole points to a valid location owned by this heap (e.g. bottom <= next < top), and that next.add(next.size) <= top.

I think this would achieve all of the goals stated by @Kazurin-775, with a single feature flag toggling the strict asserts.

As a higher cost check, we could also fill the "body" of all holes (any bytes not taken by the Hole node itself) with a magic value (similar to the technique used in "stack painting") and at allocation time - or even on every cursor traversal, ensure that all bytes are still at the expected magic value.

If you are still interested in this @Kazurin-775, I could point you how/where to make these changes.

@Kazurin-775
Copy link
Author

Sorry for the late reply. OS development is not my main focus right now, but I guess I will be able to devote some time in the next couple of months. So any suggestions and guidance are welcome! This may also help random guys on the Internet who are interested in extending the features of this crate. Thank you @jamesmunns in advance!

As a higher cost check, we could also fill the "body" of all holes with a magic value and at allocation time

I'm afraid that this is not always desirable compared to the previous ones, and it's harder to make this fit everyone's need, so we may better leave it configurable at runtime. I guess I won't implement this in the very first PR.

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

No branches or pull requests

3 participants