Dynamic code modification support for x64 NaCl modules...

src/trusted/validator_x86/ncvalidate_iter.c

Index: src/trusted/validator_x86/ncvalidate_iter.c
===================================================================
--- src/trusted/validator_x86/ncvalidate_iter.c	(revision 3655)
+++ src/trusted/validator_x86/ncvalidate_iter.c	(working copy)
@@ -14,7 +14,7 @@
 #include <string.h>
 
 #include "native_client/src/trusted/validator_x86/ncvalidate_iter.h"
-
+#include "native_client/src/shared/platform/nacl_check.h"
 #include "native_client/src/shared/platform/nacl_log.h"
 #include "native_client/src/trusted/validator_x86/nc_inst_iter.h"
 #include "native_client/src/trusted/validator_x86/nc_inst_state_internal.h"
@@ -246,10 +246,12 @@
     /* TODO(karl) - Make printing of instruction state possible via format. */
     NaClInstStateInstPrint(g, inst);
 
-    va_start(ap, format);
-    NaClLog_mu(level, "VALIDATOR: %s", NaClLogLevelLabel(level));
-    NaClLogV_mu(level, format, ap);
-    va_end(ap);
+    if (format != NULL) {

[bsy, 2010/11/11 21:27:21]

when do we get NULL format strings?  this sounds like an internal error in the
validator.  kschimpf: opinions?

[petr, 2010/11/12 18:11:06]

I use this to print two instructions in a row, see the uses below. So, my uses
are the only that give format = NULL.
I will adopt Karl's suggestion and create a separate message function

On 2010/11/11 21:27:21, bsy wrote:

+      va_start(ap, format);
+      NaClLog_mu(level, "VALIDATOR: %s", NaClLogLevelLabel(level));
+      NaClLogV_mu(level, format, ap);
+      va_end(ap);
+    }
     NaClLogUnlock();
     NaClRecordErrorReported(state, level);
   }
@@ -449,3 +451,162 @@
   }
   return NULL;
 }
+
+/*
+ * Check that iter_new is a valid replacement for iter_old.
+ * If a validation error occurs, state->validates_ok will be set to false by
+ * NaClValidatorInstMessage when it is given LOG_ERROR, see the end of this
+ * function.
+ */
+static void NaClValidateInstReplacement(NaClInstIter* iter_old,
+                                        NaClInstIter* iter_new,
+                                        struct NaClValidatorState* state) {
+  NaClInstState *istate_old, *istate_new;
+  NaClExpVector *exp_old, *exp_new;
+  uint32_t i;
+
+  istate_old = NaClInstIterGetState(iter_old);
+  istate_new = NaClInstIterGetState(iter_new);
+
+  /* Location/length must match */
+  if (istate_new->vpc != istate_old->vpc ||
+      istate_new->length != istate_old->length) {
+    NaClValidatorMessage(LOG_ERROR, state,
+        "Code modification: instructions length/addresses do not match\n");
+    NaClValidatorInstMessage(LOG_ERROR, state, istate_old, NULL);
+    NaClValidatorInstMessage(LOG_ERROR, state, istate_new, NULL);
+    return;
+  }
+
+
+  do {
+    /* fast check if the replacement is identical */
+    if (!memcmp(istate_old->mpc, istate_new->mpc, istate_old->length))
+      return;
+
+    if (istate_old->num_prefix_bytes != istate_new->num_prefix_bytes)
+      break;
+    if (istate_old->num_rex_prefixes != istate_new->num_rex_prefixes)
+      break;
+    if (istate_old->rexprefix != istate_new->rexprefix)
+      break;
+    if (istate_old->modrm != istate_new->modrm)
+      break;
+    if (istate_old->has_sib != istate_new->has_sib)
+      break;
+    if (istate_old->has_sib && istate_old->sib != istate_new->sib)
+      break;
+    if (istate_old->operand_size != istate_new->operand_size)
+      break;
+    if (istate_old->address_size != istate_new->address_size)
+      break;
+    if (istate_old->prefix_mask != istate_new->prefix_mask)
+      break;
+
+    /*
+     * these are pointers, but they reference entries in a static table,
+     * so if the two instructions are the same, then these pointers must
+     * reference the same entry
+     */
+    if (istate_old->inst != istate_new->inst)
+      break;
+
+    exp_old = NaClInstStateExpVector(istate_old);
+    exp_new = NaClInstStateExpVector(istate_new);
+
+    /* check if the instruction operands are identical */
+    if (exp_old->number_expr_nodes != exp_new->number_expr_nodes)
+      break;
+
+    for (i = 0; i < exp_old->number_expr_nodes; i++) {
+      if (exp_old->node[i].kind  != exp_new->node[i].kind)
+        break;
+      if (exp_old->node[i].flags != exp_new->node[i].flags)
+        break;
+
+      /*
+       * allow some constants to be different; however it is important not to
+       * allow modification of sandboxing instructions. Note nether of the
+       * instructions allowed for modification is used for sandboxing
+       */
+      if (exp_old->node[i].value != exp_new->node[i].value) {
+        if (exp_old->node[i].kind == ExprConstant) {
+
+          /* allow different constants in direct calls */
+          if (istate_old->inst->name == InstCall)
+            if (exp_old->node[i].flags & NACL_EFLAG(ExprJumpTarget))
+              if (exp_old->node[NaClGetExpParentIndex(exp_old, i)].kind
+                    == OperandReference)
+                continue;
+
+          /*
+           * allow different constants in operand of mov
+           * e.g. mov $rax, 0xdeadbeef
+           */
+          if (istate_old->inst->name == InstMov)
+            if (exp_old->node[i].flags & NACL_EFLAG(ExprUsed))
+              if (exp_old->node[NaClGetExpParentIndex(exp_old, i)].kind
+                    == OperandReference)
+                continue;
+          /*
+           * allow different displacements in memory reference of mov
+           * instructions e.g. mov $rax, [$r15+$rbx*2+0x7fff]
+           */
+          if (istate_old->inst->name == InstMov)
+            if (exp_old->node[NaClGetExpParentIndex(exp_old, i)].kind
+                    == ExprMemOffset)
+              /* displacement is the fourth node after ExprMemOffset node */
+              if (i - NaClGetExpParentIndex(exp_old, i) == 4)
+                continue;
+        }
+        break;
+      }
+    }
+
+    return;
+  } while (0);
+
+  NaClValidatorMessage(LOG_ERROR, state,
+        "Code modification: failed to modify instructions\n");
+  NaClValidatorInstMessage(LOG_ERROR, state, istate_old, NULL);

[bsy, 2010/11/11 21:27:21]

are these two lines the only places where the format string might be NULL?

[Karl, 2010/11/11 22:45:41]

I think that you may just want to add a new "message" function
NaClValidatorInstsMessage(....) that gets two instructions, and prints them with
a message.

[petr, 2010/11/12 18:11:06]

Yes, they are
On 2010/11/11 21:27:21, bsy wrote:

[petr, 2010/11/12 18:11:06]

Yes, I will do this

On 2010/11/11 22:45:41, Karl wrote:

+  NaClValidatorInstMessage(LOG_ERROR, state, istate_new, NULL);
+}
+
+void NaClValidateSegmentPair(uint8_t* mbase_old, uint8_t* mbase_new,
+                           NaClPcAddress vbase, size_t size,
+                           struct NaClValidatorState* state) {
+  NaClSegment segment_old, segment_new;
+  NaClInstIter *iter_old, *iter_new;
+
+  NaClValidatorStateInitializeValidators(state);
+  NaClSegmentInitialize(mbase_old, vbase, size, &segment_old);
+  NaClSegmentInitialize(mbase_new, vbase, size, &segment_new);
+  iter_old = NaClInstIterCreateWithLookback(&segment_old, kLookbackSize);
+  iter_new = NaClInstIterCreateWithLookback(&segment_new, kLookbackSize);
+  while (NaClInstIterHasNext(iter_old) &&
+         NaClInstIterHasNext(iter_new)) {
+    state->cur_inst_state = NaClInstIterGetState(iter_new);
+    state->cur_inst = NaClInstStateInst(state->cur_inst_state);
+    state->cur_inst_vector = NaClInstStateExpVector(state->cur_inst_state);
+    NaClApplyValidators(state, iter_new);
+    NaClValidateInstReplacement(iter_old, iter_new, state);
+    if (state->quit) break;
+    NaClInstIterAdvance(iter_old);
+    NaClInstIterAdvance(iter_new);
+  }
+
+  if (NaClInstIterHasNext(iter_old) ||
+      NaClInstIterHasNext(iter_new)) {
+    NaClValidatorMessage(LOG_ERROR, state,
+    "Code modification: code segments have different number of instructions\n");
+  }
+
+  state->cur_inst_state = NULL;
+  state->cur_inst = NULL;
+  state->cur_inst_vector = NULL;
+  NaClApplyPostValidators(state, iter_new);
+  NaClInstIterDestroy(iter_old);
+  NaClInstIterDestroy(iter_new);
+  NaClValidatorStatePrintStats(state);
+}
+