The sloppyRXcheck logic in the sync checker was not correct - it was

simply treating R and X as equivalent but the real problem is that
mappings can appear to have X permission entirely indepenent of anything
else with recent x86 kernels.

If a mapping is inside the (deliberately constrained) code segment then
it will appear to have X permission regardless of whether R or X was asked
for when it was mapped, so what we really need to do is allow the kernel
to add X to any mapping but not to take it away if we were expecting it.

git-svn-id: svn://svn.valgrind.org/valgrind/trunk@4985

diff --git a/coregrind/m_aspacemgr/aspacemgr.c b/coregrind/m_aspacemgr/aspacemgr.c
index 55c2a504bf4c47bafe24473c5e0b461808d692ab..600102d5149254bdf3bed3896abd985ea7f34227 100644 (file)
--- a/coregrind/m_aspacemgr/aspacemgr.c
+++ b/coregrind/m_aspacemgr/aspacemgr.c
@@ -1072,7 +1072,7 @@ static void sync_check_mapping_callback ( Addr addr, SizeT len, UInt prot,
                                           const UChar* filename )
 {
    Int  iLo, iHi, i;
-   Bool sloppyRXcheck;
+   Bool sloppyXcheck;
 
    /* If a problem has already been detected, don't continue comparing
       segments, so as to avoid flooding the output with error
@@ -1097,22 +1097,22 @@ static void sync_check_mapping_callback ( Addr addr, SizeT len, UInt prot,
    aspacem_assert(nsegments[iHi].end   >= addr + len - 1 );
 
    /* x86 doesn't differentiate 'x' and 'r' (at least, all except the
-      most recent NX-bit enabled CPUs) and so recent kernels mark most
-      readable sections also as executable (in the /proc/self/maps
-      they give out), which makes checking fail.  When sloppyRXcheck
-      is True, the checker therefore regards R and X as
-      interchangeable. */
-   sloppyRXcheck = False;
+      most recent NX-bit enabled CPUs) and so recent kernels attempt
+      to provide execute protection by placing all executable mappings
+      low down in the address space and then reducing the size of the
+      code segment to prevent code at higher addresses being executed.
+
+      These kernels report which mappings are really executable in
+      the /proc/self/maps output rather than mirroring what was asked
+      for when each mapping was created. In order to cope with this we
+      have a slopyXcheck mode which we enable on x86 - in this mode we
+      allow the kernel to report execute permission when we weren't
+      expecting it but not vice versa. */
+   sloppyXcheck = False;
 #  if defined(VGA_x86)
-   sloppyRXcheck = True;
+   sloppyXcheck = True;
 #  endif
 
-   if (sloppyRXcheck) {
-      /* If either bit is set, ensure both are set. */
-      if (prot & (VKI_PROT_READ|VKI_PROT_EXEC))
-         prot |= (VKI_PROT_READ|VKI_PROT_EXEC);
-   }
-
    /* NSegments iLo .. iHi inclusive should agree with the presented
       data. */
    for (i = iLo; i <= iHi; i++) {
@@ -1144,10 +1144,13 @@ static void sync_check_mapping_callback ( Addr addr, SizeT len, UInt prot,
       if (filename && 0==VG_(strcmp)(filename, "/dev/zero (deleted)"))
          cmp_devino = False;
 
-      if (sloppyRXcheck) {
-         /* If either bit is set, ensure both are set. */
-         if (seg_prot & (VKI_PROT_READ|VKI_PROT_EXEC))
-            seg_prot |= (VKI_PROT_READ|VKI_PROT_EXEC);
+      /* If we are doing sloppy execute permission checks then we
+         allow segment to have X permission when we weren't expecting
+         it (but not vice versa) so if the kernel reported execute
+         permission then pretend that this segment has it regardless
+         of what we were expecting. */
+      if (sloppyXcheck && (prot & VKI_PROT_EXEC) != 0) {
+         seg_prot |= VKI_PROT_EXEC;
       }
 
       same = same