Skip to content
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

xed_agen() uses current instruction address in eip relative address mode #335

Open
dougkwan opened this issue Oct 18, 2024 · 1 comment
Open

Comments

@dougkwan
Copy link

dougkwan commented Oct 18, 2024

xed_agen() uses the address of the instruction instead of the one after the instruction in eip-relative addresses. There was a similar bug for the 64-bit case with rip. It appears that the previous bug fix missed the 32-bit case. The following prorgram reproduces the bug.


#define DEBUG 1  // Cause failed assertion to abort program.
#include <assert.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/user.h>

#include "third_party/libxed/xed-interface.h"

xed_uint64_t agen_reg_callback(xed_reg_enum_t reg, void* context,
                               xed_bool_t* error) {
  *error = 0;
  struct user_regs_struct* regs = (struct user_regs_struct*)context;
  // The only register needed for the example instruction is EIP.
  switch (reg) {
    case XED_REG_EIP:
      return (uint32_t)regs->rip;
    default:
      *error = 1;
      return 0;
  }
}

xed_uint64_t agen_segment_callback(xed_reg_enum_t reg, void* context,
                                   xed_bool_t* error) {
  // Our example instruction does not use any segment register.
  *error = 1;
  return 0;
}

int main(int argc, char** argv) {
  xed_tables_init();
  xed_agen_register_callback(agen_reg_callback, agen_segment_callback);

  // mov    0x0(%eip),%al
  const static uint8_t insn_bytes[] = {0x67, 0x8a, 0x05, 0x00,
                                       0x00, 0x00, 0x00};

  xed_decoded_inst_t insn;
  xed_decoded_inst_zero(&insn);
  xed_decoded_inst_set_mode(&insn, XED_MACHINE_MODE_LONG_64,
                            XED_ADDRESS_WIDTH_64b);
  xed_error_enum_t xed_error =
      xed_decode(&insn, insn_bytes, sizeof(insn_bytes));
  assert(xed_error == XED_ERROR_NONE);
  assert(xed_decoded_inst_valid(&insn));

  xed_uint64_t address;
  struct user_regs_struct regs;
  memset(&regs, 0, sizeof(regs));
  const uint32_t kFakeRip = 0x1000;  // Must fit in 32 bits.
  regs.rip = kFakeRip;
  xed_error = xed_agen(&insn, 0, &regs, &address);
  assert(xed_error == XED_ERROR_NONE);

  // The expected value of address 0(%eip) is kFakeRip + 7.
  uint64_t expected_address = kFakeRip + xed_decoded_inst_get_length(&insn);
  printf("address = 0x%lx, expected = 0x%lx\n", address, expected_address);

  return address == expected_address ? EXIT_SUCCESS : EXIT_FAILURE;
}

@dougkwan
Copy link
Author

I adapted the 64-bit to fix the 32-bit case and it seems to be okay:

--- a/xed/src/dec/xed-agen.c	2023-10-16 11:59:57.000000000 +0000
+++ b/xed/src/dec/xed-agen.c	2024-10-17 22:50:17.403914144 +0000
@@ -128,10 +128,17 @@
     }
     else if (addr_width == 32) {
         xed_uint32_t base32 = base_value;
-        xed_uint32_t index32 = index_value;
         xed_uint32_t disp32 = XED_STATIC_CAST(xed_uint32_t, displacement);
-        xed_uint32_t ea32 = base32  + index32 * scale + disp32;
-        xed_uint32_t lin32 = segment_base + ea32;
+        xed_uint32_t lin32 = 0;
+        if (base_reg == XED_REG_EIP) {
+            xed_uint32_t inst_len =  xed_decoded_inst_get_length(xedd);
+            lin32 = base32 + inst_len + displacement;
+        }
+        else {
+            xed_uint32_t index32 = index_value;
+            xed_uint32_t ea32 = base32  + index32 * scale + disp32;
+            lin32 = segment_base + ea32;
+        }
         out = lin32;
         // FIXME: big real mode!
     }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant