Hi Linus, On Tue, Oct 17, 2023 at 10:43 AM Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > > On Tue, 17 Oct 2023 at 10:04, Linus Torvalds > <torvalds@xxxxxxxxxxxxxxxxxxxx> 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
