Hi Linus,
On Tue, Oct 17, 2023 at 10:43 AM Linus Torvalds torvalds@linux-foundation.org wrote:
On Tue, 17 Oct 2023 at 10:04, Linus Torvalds torvalds@linux-foundation.org wrote:
Honestly, there is only two kinds of sealing that makes sense:
you cannot change the permissions of some area
you cannot unmap an area
Actually, I guess at least theoretically, there could be three different things:
- you cannot move an area
Yes.
Actually, the newly added selftest covers some of the above: 1. can't change the permission of some areas. test_seal_mprotect test_seal_mmap_overwrite_prot
2. can't unmap an area (thus mmap() to the same address later) test_seal_munmap
3. can't move to an area: test_seal_mremap_move //can't move from a sealed area: test_seal_mremap_move_fixed_zero //can't move from a sealed area to a fixed address test_seal_mremap_move_fixed //can't move to a sealed area.
4 can't expand or shrink the area: test_seal_mremap_shrink test_seal_mremap_expand
although I do think that maybe just saying "you cannot unmap" might also include "you cannot move".
But I guess it depends on whether you feel it's the virtual _address_ you are protecting, or whether it's the concept of mapping something.
I personally think that from a security perspective, what you want to protect is a particular address. That implies that "seal from unmapping" would thus also include "you can't move this area elsewhere".
But at least conceptually, splitting "unmap" and "move" apart might make some sense. I would like to hear a practical reason for it, though.
Without that practical reason, I think the only two sane sealing operations are:
SEAL_MUNMAP: "don't allow this mapping address to go away"
IOW no unmap, no shrinking, no moving mremap
SEAL_MPROTECT: "don't allow any mapping permission changes"
I agree with the concept in general. The separation of two seal types is easy to understand.
For mmap(MAP_FIXED), I know for a fact that it can modify permission of an existing mapping, (as in selftest:test_seal_mmap_overwrite_prot). I think it can also expand an existing VMA. This is not a problem, code-wise, I mention it here, because it needs extra care when coding mmap() change.
Again, that permission case might end up being "don't allow _additional_ permissions" and "don't allow taking permissions away". Or it could be split by operation (ie "don't allow permission changes to writability / readability / executability respectively").
Yes. If the application desires this, it can also be done. i.e. seal of X bit, or seal of W bit, this will be similar to file sealing. I discussed this with Stephan before, at this point of time, Chrome doesn't have a use case.
I suspect there isn't a real-life example of splitting the SEAL_MPROTECT (the same way I doubt there's a real-life example for splitting the UNMAP into "unmap vs move"), so unless there is some real reason, I'd keep the sealing minimal and to just those two flags.
I think two seal-type (permission and unmap/move/expand/shrink) will work for the Chrome case. Stephen Röttger is an expert in Chrome, on vacation/ be back soon. I will wait for Stephen to confirm.
We could always add more flags later, if there is a real use case (IOW, if we start with "don't allow any permission changes", we could add a flag later that just says "don't allow writability changes").
Agreed 100%, thanks for understanding.
-Jeff
Linus