Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Complete madvise values #133

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

allan2
Copy link
Collaborator

@allan2 allan2 commented Dec 13, 2024

This PR implements all remaining madvise values specified in the Linux man page.

The following values are added:

  1. SOFT_OFFLINE
  2. WIPEONFORK
  3. KEEPONFORK
  4. COLD
  5. PAGEOUT
  6. COLLAPSE

Values 1-5 were mentioned in a comment for future expansion. COLLAPSE was also found to be missing.

All six values are memory safe. Brief explanations are below:

  1. SOFT_OFFLINE: effects do not change the semantics of the calling process.
  2. WIPEONFORK: only affects child processes by zeroing memory post-fork.
  3. KEEPONFORK: only affects child processes by preserving memory across fork.
  4. COLD: indicates pages are unlikely to be accessed soon; non-destructive.
  5. PAGEOUT: requests page reclamation; non-destructive.
  6. COLLAPSE: consolidates pages into THPs; non-destructive.

I encourage reviewers to verify my safety assumptions.

This PR implements all remaining madvise values specified in the
[Linux man page](https://man7.org/linux/man-pages/man2/madvise.2.html).

The following values are added:
1. SOFT_OFFLINE
2. WIPEONFORK
3. KEEPONFORK
4. COLD
5. PAGEOUT
6. COLLAPSE

Values 1-5 were mentioned in a comment for future expansion.
COLLAPSE was also found to be missing.

All six values are memory safe. Brief explanations are below:

1. SOFT_OFFLINE: effects do not change the semantics of the calling process.
2. WIPEONFORK: only affects child processes by zeroing memory post-fork.
3. KEEPONFORK: only affects child processes by preserving memory across fork.
4. COLD: indicates pages are unlikely to be accessed soon; non-destructive.
5. PAGEOUT: requests page reclamation; non-destructive.
6. COLLAPSE: consolidates pages into THPs; non-destructive.

I encourage reviewers to verify my safety assumptions.
This fixes the musl build by excluding MADV_COLLAPSE under musl.
@de-vri-es
Copy link
Collaborator

de-vri-es commented Dec 13, 2024

Is WIPEONFORK really safe? It is possible to execute code after fork() before exec() using Command::pre_exec()

pre_exec() is unsafe itself, but using WIPEONFORK can make things that would otherwise be sound unsound. I'm not sure if this is a problem in combination with memory maps, but I would be hesitant to call it safe.

@allan2
Copy link
Collaborator Author

allan2 commented Jan 20, 2025

I tested WIPEONFORK here --> https://github.com/allan2/wipeonfork-pre-exec-example
Memory is zeroed both in the child process and before exec when using pre_exec.

This should cover the typical use case. However, there is still potential for unsafety if pre_exec logic assumes pre-fork memory state. If you are still concerned, I can move the variant to UncheckedAdvice.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants