On Fri, Feb 2, 2024 at 5:59 PM Jeff Xu jeffxu@chromium.org wrote:
On Thu, Feb 1, 2024 at 9:00 PM Theo de Raadt deraadt@openbsd.org wrote:
Jeff Xu jeffxu@chromium.org wrote:
Even without free. I personally do not like the heap getting sealed like that.
Component A. p=malloc(4096); writing something to p.
Compohave nent B: mprotect(p,4096, RO) mseal(p,4096)
This will split the heap VMA, and prevent the heap from shrinking, if this is in a frequent code path, then it might hurt the process's memory usage.
The existing code is more likely to use malloc than mmap(), so it is easier for dev to seal a piece of data belonging to another component. I hope this pattern is not wide-spreading.
The ideal way will be just changing the library A to use mmap.
I think you are lacking some test programs to see how it actually behaves; the effect is worse than you think, and the impact is immediately visible to the programmer, and the lesson is clear:
you can only seal objects which you gaurantee never get recycled. Pushing a sealed object back into reuse is a disasterous bug. Noone should call this interface, unless they understand that.I'll say again, you don't have a test program for various allocators to understand how it behaves. The failure modes described in your docuemnts are not correct.
I understand what you mean: I will add that part to the document: Try to recycle a sealed memory is disastrous, e.g. p=malloc(4096); mprotect(p,4096,RO) mseal(p,4096) free(p);
My point is: I think sealing an object from the heap is a bad pattern in general, even dev doesn't free it. That was one of the reasons for the sealable flag, I hope saying this doesn't be perceived as looking for excuses.
The point you're missing is that adding MAP_SEALABLE reduces composability. With MAP_SEALABLE, everything that mmaps some part of the address space that may ever be sealed will need to be modified to know about MAP_SEALABLE.
Say you did the same thing for mprotect. MAP_PROTECT would control the mprotectability of the map. You'd stop:
p = malloc(4096); mprotect(p, 4096, PROT_READ); free(p);
! But you'd need to change every spot that mmap()'s something to know about and use MAP_PROTECT: all "producers" of mmap memory would need to know about the consumers doing mprotect(). So now either all mmap() callers mindlessly add MAP_PROTECT out of fear the consumers do mprotect (and you gain nothing from MAP_PROTECT), or the mmap() callers need to know the consumers call mprotect(), and thus you introduce a huge layering violation (and you actually lose from having MAP_PROTECT).
Hopefully you can map the above to MAP_SEALABLE. Or to any other m*() operation. For example, if chrome runs on an older glibc that does not know about MAP_SEALABLE, it will not be able to mseal() its own shared libraries' .text (even if, yes, that should ideally be left to ld.so).
IMO, UNIX API design has historically mostly been "play stupid games, win stupid prizes", which is e.g: why things like close(STDOUT_FILENO) work. If you close stdout (and don't dup/reopen something to stdout) and printf(), things will break, and you get to keep both pieces. There's no O_CLOSEABLE, just as there's no O_DUPABLE.