Add mlockall and munlockall
#872
Conversation
newpavlov
force-pushed
the
mlockall
branch
5 times, most recently
from
eefd082 to
b20d197
Compare
| /// [`mlockall`]: crate::mm::mlockall | ||
| pub struct MlockallFlags: i32 { | ||
| // libc doesn't define `MCL_ONFAULT` yet. | ||
| // const ONFAULT = libc::MCL_ONFAULT; |
There was a problem hiding this comment.
newpavlov
force-pushed
the
mlockall
branch
6 times, most recently
from
48bd77b to
970d43e
Compare
| target_os = "dragonfly", | ||
| target_os = "illumos", | ||
| ))] | ||
| pub fn munlockall() -> io::Result<()> { |
There was a problem hiding this comment.
Currently, munlock is unsafe, because it takes a raw pointer and can operate on memory the caller doesn't own or borrow. It doesn't segfault or access data, but it can potentially compromise the security assumptions of the code that does own the memory.
Should munlockall be unsafe too? On one hand, it does a superset of what munlock does, which seems to suggest that it should be unsafe.
But on the other, since munlockall doesn't take a pointer, it somehow has to know where all the allocated pages in the process are, which is something that can really only be done by debuggers or similar. And in Rust, debuggers do not make things unsafe. open isn't unsafe just because you can open /proc/self/mem; that's considered a debugging interface. So in principle, munlockall is like attaching a debugger to the process and doing magic. You can do it, but what it does is outside the scope of Rust semantics.
I'm open to other opinions here, but right now I think I buy this argument that munlockall is like a debugger, and therefore can be a safe function. But that said, we should document this.
The comment should say something along the lines of "warning, this function is aware of all the memory pages in the process, as if it were a debugger. It unlocks all the pages, which could potentially compromise security assumptions made by code about memory it has encapsulated".
There was a problem hiding this comment.
I will add a warning comment, but I don't think that mlockall and munlockall should be marked unsafe. In my understanding, unsafe should be used only for code which could violate Rust memory safety and swapping is far outside of the Rust memory model. I vaguely remember discussions where overwhelming opinion was that using unsafe for code which may cause security issues is a misuse of unsafe.
There was a problem hiding this comment.
Yes, I ultimately also concluded that they should be safe too, though for slightly different reasons.
I'm aware of the discussions. I believe the strong opinions about this are motivated by the perpetual need to push back against the endless stream of miscellaneous invariants that might get added to unsafe otherwise, and I broadly agree.
My interpretation of memory safety is that it's about anything that bypasses the Rust memory abstraction level, and instead operates at the level of "a pointer is just an integer, memory is just a global array of bytes". This is a subtle and debatable distinction, but my main point here is just that I don't see this as disagreeing with the strong opinions, it's just a specific interpretation of them.
Separately, "/proc/self/mem", teaches us that memory safety is not the only question. We also need to ask something like "is it a debugger?". That's also a subtle question, but my point here is just to say that in this moment, in this context, I think munlockall qualifies as a debugger, which makes it safe (but still deserves documentation comments).
newpavlov
force-pushed
the
mlockall
branch
5 times, most recently
from
5b5e0e0 to
4902d3f
Compare
|
The 1.63 build error in CI should be fixed on main, so don't worry about that. |
|
Looks good! |
No description provided.