On Tue, May 14, 2024 at 09:14:37PM -0700, Linus Torvalds wrote: > On Tue, 14 May 2024 at 20:36, Linus Torvalds > <torvalds@xxxxxxxxxxxxxxxxxxxx> wrote: > > > > Guys, if you let untrusted code execute random system calls, the whole > > "look, now unmap() acts oddly" IS THE LEAST OF YOUR ISSUES. I totally agree with this, I'm more speaking about a more general hardening measure, like what is commonly offered via prctl() and that, for example, manages to mitigate the consequences of a successful RCE. > Side note: it doesn't even help to make things "atomic". munmap() acts > oddly whether it fals completely or whether it fails partially, and if > the user doesn't check the result, neither case is great. I don't find the "atomic" aspect that important either, however the munmap() man page says: All pages containing a part of the indicated range are un- mapped, and subsequent references to these pages will generate SIGSEGV. It is not an error if the indicated range does not contain any mapped pages. This alone is an encouragement to not check the result. And BTW, what should one do to try to repair the situation after a failed munmap() ? It reads as "best effort" above: usually upon return, anything that could be unmapped was unmapped. That's how I'm reading it. I think it's a nice property that makes this syscall trustable by its users, and contrary to the atomic aspect I would find it nice if munmap() would properly unmap everything it can then return the error caused by the encounter of a sealed area. For me that's even the opposite of an atomic approach, it's really about making sure to best follow the developer's intent regardless of any obstacles. > If you want to have some "hardened mseal()", you make any attempt to > change a mseal'ed memory area be a fatal error. The whole "atomic or > not" is a complete red herring. Yep, agreed. > I'd certainly be ok with that. If the point of mseal is "you can't > change this mapping", then anybody who tries to change it is obviously > untrustworthy, and killing the whole thing sounds perfectly sane to > me. > > Maybe that's a first valid use-case for the flags argument. That could be for that use case (developer doing mseal, attacker trying munmap), indeed, though that couldn't cover for the other way around (attacker doing mseal() in hope to make a future munmap() fail). That's what I like with prctl(), it offers the developer a panoply of options to decide when and how to lock down a process in order to mitigate consequences of exploited bugs. And it could be independent on this series, by essentially focusing on the ability to kill a process that fails to munmap() a sealed area. I.e. no need to that that property on the area itself, it's a matter of whether we consider the process sensitive enough or not. Willy
