Skip to content

Despite dual mapping the VM is still flipping page protection bits on executable pages #37739

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mkustermann opened this issue Aug 5, 2019 · 10 comments
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. vm-gc Related to the VM's garbage collector

Comments

@mkustermann
Copy link
Member

Despite the fact that the VM can run with dual mapping enabled, we still seem to flip page protection bits on the executable pages in GC related code.

For example runtime/vm/heap/pages.cc:

HeapPage* PageSpace::AllocatePage(HeapPage::PageType type, bool link) {
    ...
      if (exec_pages_ == NULL) {
        exec_pages_ = page;
      } else {
        if (FLAG_write_protect_code) {
          exec_pages_tail_->WriteProtect(false);
        }
        exec_pages_tail_->set_next(page);
        if (FLAG_write_protect_code) {
          exec_pages_tail_->WriteProtect(true);
        }
    ...
}

The GC should use the RW mapping for performing these writes instead of flipping the protection on the RX mapping (which is more expensive due to performing syscalls).

/cc @rmacnak-google @crelier

@mkustermann mkustermann added the area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. label Aug 5, 2019
@crelier
Copy link
Contributor

crelier commented Aug 5, 2019

As far as I can tell, this bit flipping is applied to the read/write mapping of code pages, not to the executable mapping (this would fail on Fuchsia, since the executable mapping does does have a write capability).
Although we use dual mapping, we still protect the writable mapping.
Think of dual mapping as an additional security measure. It does not relax the security of the read/write mapping by leaving it writable at all times.

@mkustermann
Copy link
Member Author

Although we use dual mapping, we still protect the writable mapping.

Is this necessary?

If I understand this correctly: The addresses for the code we execute might leak to an adversary, so we want to ensure that those addresses are never writable. So we map the writable part in a place in the address space which is hard for an adversary to get addresses to.

We could benefit with better performance by not flipping the protection bits on writable mappings: Any allocation of an RawInstructions object forces us to stop the mutator. The mutator then has to wait fo free list traversals of the BG compiler, some mprotect calls until the instruction object is finally created, initialized and installed on the function.

@sjindel-google
Copy link
Contributor

The original problem was whether the executable mappings will always be executable. This is all that matters for https://dart-review.googlesource.com/c/sdk/+/111280

@mkustermann
Copy link
Member Author

(@sjindel-google The original problem has nothing to do with FFI. This is something we hit with our concurrency work.)

Although we use dual mapping, we still protect the writable mapping.

Never mind my comment above. Thinking about this again, with dual mapping we actually don't necessarily have to be at a safepoint anymore when allocating RawInstructions. Though there needs to be some coordination between multiple threads allocating executable memory and filling it in and the GC.

Thanks, Regis!

@crelier
Copy link
Contributor

crelier commented Aug 6, 2019

Yes, that is correct, flipping the bit on the writable mapping offers a little more security. But if not doing it provides a noticeable performance increase, we could try to negotiate with the security folks. I think @rmacnak-google was going to do an experiment to see if this is worth it.

To answer @sjindel-google, the executable mapping is readonly while the instructions are being written in the writeable mapping. Then, the executable mapping is flipped to read/executable and should stay in that state. I am pretty sure that it is not flipped back to readonly at any time, but I could also be wrong :-)

@mkustermann
Copy link
Member Author

To answer @sjindel-google, the executable mapping is readonly while the instructions are being written in the writeable mapping. Then, the executable mapping is flipped to read/executable and should stay in that state.

What happens if we allocate another instructions object in the same heap page? Does that keep the RX in executable mapping and only uses the RW mapping for writing? Looking at Code::FinalizeCode it seems to change protection in both mappings. That would affect any other instructions objects on that page as well, wouldn't it?

@crelier
Copy link
Contributor

crelier commented Aug 6, 2019

Note that the protection is changed for the length of the instructions object and not for the whole heap page. Unless the new instructions object is overlapping the existing one, the protection of the existing one is not changed.

@mkustermann
Copy link
Member Author

Note that the protection is changed for the length of the instructions object and not for the whole heap page.

The granularity on which protections can be set is OS page level, mostly 4 kb and we can have multiple instructions objects in one OS page.

@mkustermann
Copy link
Member Author

See cl/112244, which would change this behavior. If you are all happy with this

(From a security point of view I think it makes little difference, since the pages are 0 filled from start. After the initial instructions object is in there it's RX and any further code collections and re-allocations happen then in RX region. So we might as well make it RX from the start)

dart-bot pushed a commit that referenced this issue Aug 8, 2019
…iately

Currently the initial mapping for the executable mapping is read-only.
Once the first instruction object was allocated into an OS page we would
map that page as RX. Any further allocations of instructions objects
into the same page would just end up mapping it to RX again (even though
it is already that way).

To avoid those additional protection calls we can map the executable
mapping RX from the beginning (it will be filled with zeros after
allocation).

Issue #37739
Issue #36097

Change-Id: Ib83f0be8ea8dacc86646c0a3c0335f4886516caa
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/112244
Commit-Queue: Martin Kustermann <[email protected]>
Reviewed-by: Régis Crelier <[email protected]>
@mraleph
Copy link
Member

mraleph commented Jan 9, 2024

Dual mapping was removed.

@mraleph mraleph closed this as completed Jan 9, 2024
@rmacnak-google rmacnak-google added the vm-gc Related to the VM's garbage collector label Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-vm Use area-vm for VM related issues, including code coverage, and the AOT and JIT backends. vm-gc Related to the VM's garbage collector
Projects
None yet
Development

No branches or pull requests

5 participants