Bugzilla

(In reply to Cristiano Giuffrida from comment #7)
> The remaining way syscalls like mprotect() can kill the signal is to use a serializing instruction somewhere. In the case of mprotect(), the mandatory "invlpg" instruction does the trick (as Hany & Enrico confirmed experimentally---this behavior is also explicitly documented in Intel/AMD manuals).

Thanks, that's good to know because this applies to us.

> In short, to implement a future-proof mitigation strategy, IMO a good option is to simply place a cheap lfence instruction (or similar) in the FlushICache() function. Or alternatively, if all the code paths will always go through a mprotect() syscall by design, explicitly document that such paths rely on the mprotect() code itself (e.g., invlpg) to serialize the execution and kill the signal.

We have just [one caller](https://searchfox.org/mozilla-central/rev/0379f315c75a2875d716b4f5e1a18bf27188f1e6/js/src/jit/ProcessExecutableMemory.cpp#745) of `FlushICache`, right before we do the `VirtualProtect/mprotect`. This was by design: if we flush when we make code executable, we can never forget or miss an ICache flush.

Furthermore, looking at `ReprotectRegion`, we already [do this](https://searchfox.org/mozilla-central/rev/0379f315c75a2875d716b4f5e1a18bf27188f1e6/js/src/jit/ProcessExecutableMemory.cpp#775) right before the `mprotect`:
```cpp
  std::atomic_thread_fence(std::memory_order_seq_cst);
```
I verified this ends up compiling to an `mfence` on x64. So I think we're good here for these two reasons.

(In reply to Cristiano Giuffrida from comment #7)
> The remaining way syscalls like mprotect() can kill the signal is to use a serializing instruction somewhere. In the case of mprotect(), the mandatory "invlpg" instruction does the trick (as Hany & Enrico confirmed experimentally---this behavior is also explicitly documented in Intel/AMD manuals).

Thanks, that's good to know because this applies to us.

> In short, to implement a future-proof mitigation strategy, IMO a good option is to simply place a cheap lfence instruction (or similar) in the FlushICache() function. Or alternatively, if all the code paths will always go through a mprotect() syscall by design, explicitly document that such paths rely on the mprotect() code itself (e.g., invlpg) to serialize the execution and kill the signal.

We have just [one caller](https://searchfox.org/mozilla-central/rev/0379f315c75a2875d716b4f5e1a18bf27188f1e6/js/src/jit/ProcessExecutableMemory.cpp#745) of `FlushICache`, right before we do the `VirtualProtect/mprotect`. This was by design: if we flush when we make code executable, we can never forget or miss an ICache flush.

Furthermore, looking at `ReprotectRegion`, we already [do this](https://searchfox.org/mozilla-central/rev/0379f315c75a2875d716b4f5e1a18bf27188f1e6/js/src/jit/ProcessExecutableMemory.cpp#775) right before the `mprotect`:
```cpp
  std::atomic_thread_fence(std::memory_order_seq_cst);
```
I verified this ends up compiling to an `mfence` on x64. So I think we're good here for two separate reasons.