postmodern
commented
2 years ago Just chiming in with a +1 after skimming through the code and noticed PROT_WRITE was left on. W^X memory pages help mitigate (read: increase difficulty, not necessarily eliminate) against JIT security vulnerabilities where an attacker somehow manages to get arbitrary data copied into the JIT's executable memory pages, which then eventually gets executed resulting in RCE. Disabling PROT_WRITE after YJIT has written to the executable memory page would also harden YJIT against any potential memory corruption bugs where non-JIT data somehow accidentally gets written to the YJIT memory page and corrupts the emitted ASM opcodes.
maximecb
Context: https://bugs.ruby-lang.org/issues/18260
In order to respect BSD's memory mapping restrictions, we'd need to only set memory as writable or executable, but not both at the same time. Apparently, this policy is becoming more common, so it would be good to address this, as it might affect more than BSD systems.
I think what we could do, is when we initially allocate executable memory, we could set it to executable (but not writable). Then, when we generate code, we could set the memory to writable but not executable, and flip this back when leaving YJIT. Could be slightly tricky because we're probably patching code in a few different places (when doing GC, for example).
Not sure what the performance impact of this will be. I anticipate it won't be catastrophic because we only compile ~70K blocks for SFR. If we can be reasonably smart and not call mprotect too much more often than necessary, it should be OK.
Tagging @XrXr to make you aware of this issue. Not sure if we should try to fix this now or try to address that at the same time as code GC. If we don't fix it now, we might want to disable YJIT on BSD if there's any issues upstream.