Do not map read-only data memory sections with EXECUTE flags.

[digit]

Summary

The code in SectionMemoryManager.cpp unnecessarily maps
read-only data sections with the READ+EXECUTE flags. This is
undesirable from a security stand-point.

Moreover, on the Fuchsia platform, which is now very strict
about mapping pages with the EXECUTE permission, this simply
fails, because the section's pages were initially allocated
with only the READ+WRITE flags.

A more detailed description of the issue can be found in this
public SwiftShader bug:

  
https://issuetracker.google.com/issues/154586551

This patch just restrict the mapping to the READ flag for ROData
sections. Code sections are still mapped with READ+EXECUTE as
expected.

[digit]

Being very new here, I'm not sure I understand the build failures reported here, since it looks like they are entirely unrelated to this patch.
Is there anything I should do to fix this? retry the build? or just wait?

[digit]

Friendly ping?

[capn]

Ping. Would really love to know whether this execute permission serves a purpose or whether it's a copy-paste bug.

[sanjoy]

Andy Kaylor added this functionality in a commit from 2012. So it is better if Andy reviews the change.

[capn]

@andrew.w.kaylor Please take a look at this security issue.

[lhames]

I don't see any reason for RODataMem to be mapped executable. I don't think this will break anything, but if it does the fix would be to update RuntimeDyld to ensure any code that needs executing ends up in CodeMem.

LGTM.

[lhames]

@digit -- Do you have commit access to land this, or would you like me to commit on your behalf?

llvm/lib/ExecutionEngine/SectionMemoryManager.cpp

bool SectionMemoryManager::finalizeMemory(std::string *ErrMsg) {
   if (ec) {
     if (ErrMsg) {
       *ErrMsg = ec.message();
     }
     return true;
   }
 
   // Make read-only data memory read-only.
-  ec = applyMemoryGroupPermissions(RODataMem,
-                                   sys::Memory::MF_READ | sys::Memory::MF_EXEC);
+  ec = applyMemoryGroupPermissions(RODataMem, sys::Memory::MF_READ);
   if (ec) {
     if (ErrMsg) {
       *ErrMsg = ec.message();
     }
     return true;
   }
 
   // Read-write data memory already has the correct permissions