Linus Torvalds torvalds@linux-foundation.org wrote:
On Tue, 17 Oct 2023 at 11:20, Theo de Raadt deraadt@openbsd.org wrote:
The only case where the immutable marker is ignored is during address space teardown as a result of process termination.
.. and presumably also execve()?
Ah yes of course that also.
I do like us starting with just "mimmutable()", since it already exists. Particularly if chrome already knows how to use it.
Well, our chrome fork knows how to use it. Robert Nagy in our group maintains 1280 patches to make chrome work on OpenBSD. Google ignores them and will not upstream them. Some of these changes are security related, and they ignore them. Other changes are to cope with security work we've done on our own, for example: JIT changes from Stephen@google for mandatory IBT which google hasn't upstreamed yet, impacts due to PROT_EXEC-only mappings, etc.
But the only chrome diff required for mimmutable is for that v8_flags thing I described. And the same issue would need handling for mseal(). Introducing the new "mutable" ELF section is probably going to be a bigger fuss than the system call after mprotect(PROT_READ)....
Maybe add a flag field (require it to be zero initially) just to allow any future expansion. Maybe the chrome team has *wanted* to have some finer granularity thing and currently doesn't use mimmutable() in some case?
There's only one feature I can think of, and we already do it right now, documented in our manual page:
CAVEATS At present, mprotect(2) may reduce permissions on immutable pages marked PROT_READ | PROT_WRITE to the less permissive PROT_READ. This one-way operation is permitted for an introductory period to observe how software uses this mechanism. It may change to require explicit mutable region annotation with __attribute__((section(".openbsd.mutable"))) and explicit calls to mimmutable().
We had something which needed this behaviour during the development transition. It is exlusively mprotect RW -> R, no other transitions allowed.
But once the transition was done, we don't need it anymore. I want to delete it, because it is a bit of a trap. It still fails closed from an attack perspective, but...
What worries me is a piece of code reached by mistake can do a mprotect(lowering), not receive -1 EPERM, and carry on running.. I'd prefer the first time you touch a mapping in the wrong way, you receive indication of error. This only applies applies to mprotect() acting up on a region so the argument is a bit weak, due to mprotect() return value checking being about as rare as unicorns.
Also, it would be a pain for OpenBSD to transition to adding a 0 flag. I would need to see real cause not theory.