-
Notifications
You must be signed in to change notification settings - Fork 102
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
Surround memory mappings with guard pages #77
Conversation
In theory it is possible to take a pointer to a `MmapedRegion` and read/write outside of the region's bounds. We try to prevent this behavior by surrounding the underlying mapped memory with guard pages. Signed-off-by: Serban Iorga <[email protected]>
@bonzini @jiangliu @alexandruag Sorry, just a quick ping. Can you take a look on this proposal ? Thanks ! |
Will these red zones cause trouble to huge page mappings? |
@jiangliu I don't think huge page mappings can lead to corner cases here. So I can't think of any trouble. I hope I'm not missing anything. |
Hi @bonzini, @jiangliu, @alexandruag, When you have a few minutes to spare could you help me with a quick high-level opinion here ? I would just like to understand if there is any interest in this feature or if you think that it would bring enough value compared to the added complexity in order to pursue it. This PR is only a draft, but if there's any interest I can spend some more time and try to clean up the implementation. If not, I can just close it. Thanks, |
Have you considered the case of jumping over the guard pages? Although the possibility is less due to correct bound checking but there were vulnerabilities related to this in linux kernel. Even though stack based arrays aren't widely used in rust but I'm not sure if rust has equivalent of GCC's |
return false; | ||
} | ||
|
||
let page_size = match unsafe { libc::sysconf(libc::_SC_PAGESIZE) } { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we align the redzone to hugepage size(2M/1G)?
Otherwise it may conflicts with THP.
Could you please help add some unit tests for the new interface? |
@glitzflitz thanks for the comment. It's true that this protection doesn't cover all the scenarios. It's always possible to jump over the guard pages. But it makes it less likely to break out of the memory region especially by accident. I am not very familiar with @jiangliu thank you for the review. As soon as I can I will take another look on this draft PR, try to simplify it if possible, and also address your comments. |
Hi, apologies for the delayed response. I'll have a better look at the code a bit later, but right now it seems like a less invasive option is to leverage the My current preference is to see what can be done in terms of simplications/performance improvements before changing how other things work (for example, if it makes sense to restrict the host addresses of the mapped regions to get contiguous areas, that might simplify using guard pages as well). Does the |
@alexandruag Thanks for the suggestion. If I understand correctly |
Hey ! As suggested by @alexandruag after a new version of vm-memory is released, I will use |
implements #62
In theory it is possible to take a pointer to a
MmapedRegion
and read/write outside of the region's bounds. We can try to prevent this behavior by surrounding the underlying mapped memory with guard pages.Posting this as a draft in order to understand if this feature is of any interest to the community and to the maintainers of the crate.