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

Make mmap usage safe/sound #667

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

CraftSpider
Copy link
Contributor

For those not aware - mmap is very difficult to use soundly, and relies on other programs in the file system not doing things like truncating the file beneath you. It's also not actually always faster. As such, it's often better to simply use an in-memory buffer.

Not sure if there's desire for this change, but it was easy enough, so figured I'd put it up for review.

For those not aware - mmap is very difficult to use soundly, and relies on other programs in the file system not doing things like truncating the file beneath you. It's also not actually always faster. As such, it's often better to simply use an in-memory buffer.
@bjorn3
Copy link
Member

bjorn3 commented Sep 7, 2024

This doesn't help soundness at all. All files backtrace-rs mmap's are mmaped by the dynamic linker or the kernel anyway for the executable code of the process and thus modifying them would cause crashes or UB anyway.

@CraftSpider
Copy link
Contributor Author

Ah, I didn't consider that. That's good to know - instead of removing it, if you'd like, I can add actual soundness preconditions and a safety comment using that justification for it being okay.

@workingjubilee
Copy link
Member

Yeah, let's document this properly then.

@workingjubilee
Copy link
Member

...actually, what bjorn3 said raises a larger issue, too...

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

Successfully merging this pull request may close these issues.

3 participants