Merge 7712 - Ensure super instructions are marked during dynamic code modification.

src/trusted/validator/x86/ncval_seg_sfi/ncvalidate.c

 /*
  * Copyright (c) 2011 The Native Client Authors. All rights reserved.
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 /*
  * ncvalidate.c
  * Validate x86 instructions for Native Client
  *
@@ skipping 387 matching lines @@
     vprint(vstate, (reporter,
                     "RememberIP: Saw inst at %"NACL_PRIxNaClPcAddressAll
                     " twice\n", ip));
     NCStatsInternalError(vstate);
     return;
   }
   NCStatsInst(vstate);
   NCSetAdrTable(ioffset, vstate->vttable);
 }
 
-static void RememberTP(const NaClPcAddress src, NaClPcAddress target,
-                       struct NCValidatorState *vstate) {
+static void RememberTP(const NCDecoderInst *dinst, const NaClPcAddress src,
+                       NaClPcAddress target, struct NCValidatorState *vstate) {
   const NaClMemorySize ioffset =  target - vstate->iadrbase;
 
   if (NCAddressInMemoryRange(target, vstate)) {
     NCSetAdrTable(ioffset, vstate->kttable);
   }
   else if ((target & vstate->alignmask) == 0) {
     /* Allow bundle-aligned jumps. */
-  }
-  else {
+  } else if (dinst->unchanged) {
+    /* If we are replacing this instruction during dynamic code modification
+     * and it has not changed, the jump target must be valid because the
+     * instruction has been previously validated.  However, we may be only
+     * replacing a subsection of the code segment and therefore may not have
+     * information about instruction boundries outside of the code being
+     * replaced. Therefore, we allow unaligned direct jumps outside of the code
+     * being validated if and only if the instruction is unchanged.
+     * If dynamic code replacement is not being performed, inst->unchanged
+     * should always be false.
+     */
+  } else {
     ValidatePrintError(src, "JUMP TARGET out of range", vstate);
     NCStatsBadTarget(vstate);
   }
 }
 
 static void ForgetIP(const NaClPcAddress ip,
                      struct NCValidatorState *vstate) {
   NaClMemorySize ioffset =  ip - vstate->iadrbase;
   if (!NCAddressInMemoryRange(ip, vstate)) {
     ValidatePrintError(ip, "JUMP TARGET out of range in ForgetIP", vstate);
@@ skipping 74 matching lines @@
   }
 }
 
 static void ValidateJmp8(const NCDecoderInst *dinst) {
   int8_t offset = NCInstBytesByteInline(&dinst->inst_bytes,
                                         dinst->inst.prefixbytes+1);
   struct NCValidatorState* vstate = NCVALIDATOR_STATE_DOWNCAST(dinst->dstate);
   NaClPcAddress target =
       dinst->vpc + dinst->inst.bytes.length + offset;
   NCStatsCheckTarget(vstate);
-  RememberTP(dinst->vpc, target, vstate);
+  RememberTP(dinst, dinst->vpc, target, vstate);
 }
 
 static void ValidateJmpz(const NCDecoderInst *dinst) {
   NCInstBytesPtr opcode;
   uint8_t opcode0;
   int32_t offset;
   NaClPcAddress target;
   NCValidatorState* vstate = NCVALIDATOR_STATE_DOWNCAST(dinst->dstate);
   NCInstBytesPtrInitInc(&opcode, &dinst->inst_bytes,
                         dinst->inst.prefixbytes);
@@ skipping 11 matching lines @@
      *    E8: call $Jz
      *    E9: jmp $Jx
      */
     NCInstBytesPtr opcode_1;
     NCInstBytesPtrInitInc(&opcode_1, &opcode, 1);
     offset = NCInstBytesInt32(&opcode_1);
     /* as a courtesy, check call alignment correctness */
     if (opcode0 == 0xe8) ValidateCallAlignment(dinst);
   }
   target = dinst->vpc + dinst->inst.bytes.length + offset;
-  RememberTP(dinst->vpc, target, vstate);
+  RememberTP(dinst, dinst->vpc, target, vstate);
 }
 
 /*
  * The NaCl five-byte safe indirect calling sequence looks like this:
  *   83 e0 e0                 and  $0xe0,%eax
  *   ff d0                    call *%eax
  * The call may be replaced with a ff e0 jmp. Any register may
  * be used, not just eax. The validator requires exactly this
  * sequence.
  * Note: The code above assumes 32-bit alignment. Change e0 as appropriate
@@ skipping 332 matching lines @@
   NCStatsUnsafeIndirect(NCVALIDATOR_STATE_DOWNCAST(dinst_new->dstate));
 }
 
 /*
  * Check that mstate_new is a valid replacement instruction for mstate_old.
  * Note that mstate_old was validated when it was inserted originally.
  */
 static Bool ValidateInstReplacement(NCDecoderStatePair* tthis,
                                     NCDecoderInst *dinst_old,
                                     NCDecoderInst *dinst_new) {
+
+
   /* Only validate individual instructions that have changed. */
-  if (memcmp(dinst_old->inst.bytes.byte,
-             dinst_new->inst.bytes.byte,
-             dinst_new->inst.bytes.length)) {
-    /* Call single instruction validator first, will call ValidateIndirect5() */
-    ValidateInst(dinst_new);
-  } else {
-    /* Still need to record there is an intruction here for NCValidateFinish()
-     * to verify basic block alignment.
-     */
-    RememberIP(dinst_new->vpc, NCVALIDATOR_STATE_DOWNCAST(dinst_new->dstate));
-  }
+  dinst_new->unchanged = memcmp(dinst_old->inst.bytes.byte,
+                                dinst_new->inst.bytes.byte,
+                                dinst_new->inst.bytes.length) == 0;
+  ValidateInst(dinst_new);
 
   if (dinst_old->opinfo->insttype == NACLi_INDIRECT
     || dinst_new->opinfo->insttype == NACLi_INDIRECT) {
     /* Verify that nacljmps never change */
     ValidateIndirect5Replacement(dinst_old, dinst_new);
   }
   return TRUE;
 }
 
 /* Create the decoder state for the validator state, using the
@@ skipping 106 matching lines @@
   /* check basic block boundaries */
   for (offset = 0; offset < vstate->iadrlimit - vstate->iadrbase;
        offset += vstate->alignment) {
     if (!NCGetAdrTable(offset, vstate->vttable)) {
       ValidatePrintError(vstate->iadrbase + offset,
                          "Bad basic block alignment", vstate);
       NCStatsBadAlignment(vstate);
     }
   }
 }