]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commitdiff
[gdb/tdep] Use std::array in amd64-windows-tdep.c
authorTom de Vries <tdevries@suse.de>
Wed, 30 Oct 2024 12:30:51 +0000 (13:30 +0100)
committerTom de Vries <tdevries@suse.de>
Wed, 30 Oct 2024 12:30:51 +0000 (13:30 +0100)
I noticed commit 84786372e1c ("Fix size of register buffer") fixing a
stack-buffer-overflow found by AddressSanitizer in
amd64_windows_store_arg_in_reg:
...
-  gdb_byte buf[8];
+  gdb_byte buf[16];
...
and wondered if we could have found this without AddressSanitizer.

I realized that the problem is that this:
...
  gdb_byte buf[N];
  ...
  regcache->cooked_write (regno, buf);
...
is using the deprecated variant of cooked_write instead of the one using
gdb::array_view:
...
  /* Transfer of pseudo-registers.  */
  void cooked_write (int regnum, gdb::array_view<const gdb_byte> src);

  /* Deprecated overload of the above.  */
  void cooked_write (int regnum, const gdb_byte *src);
...
and consequently cooked_write does not know the size of buf.

Fix this by using std::array, and likewise in other places in
gdb/amd64-windows-tdep.c.

In the process I fixed another out of bounds access here:
...
gdb_byte imm16[2];
  ...
cache->prev_sp = cur_sp
  + extract_unsigned_integer (imm16, 4, byte_order);
...
where we're reading 4 bytes from the 2-byte buffer imm16.

Tested by rebuilding on x86_64-linux.

Tested-By: Hannes Domani <ssbssa@yahoo.de>
gdb/amd64-windows-tdep.c

index 57dcc4f56bda285d773b8bbf425dc38d9b25cee3..df59a44b824e118ccaded1683a9d975890bb8d6c 100644 (file)
@@ -208,12 +208,15 @@ amd64_windows_store_arg_in_reg (struct regcache *regcache,
   struct type *type = arg->type ();
   const gdb_byte *valbuf = arg->contents ().data ();
   /* We only set 8 bytes, buf if it's a XMM register, 16 bytes are read.  */
-  gdb_byte buf[16];
+  std::array<gdb_byte, 16> buf;
 
   gdb_assert (type->length () <= 8);
-  memset (buf, 0, sizeof buf);
-  memcpy (buf, valbuf, std::min (type->length (), (ULONGEST) 8));
-  regcache->cooked_write (regno, buf);
+  memset (buf.data (), 0, buf.size ());
+  memcpy (buf.data (), valbuf, std::min (type->length (), (ULONGEST) 8));
+  size_t reg_size = regcache_register_size (regcache, regno);
+  gdb_assert (reg_size <= buf.size ());
+  gdb::array_view<gdb_byte> view (buf);
+  regcache->cooked_write (regno, view.slice (0, reg_size));
 }
 
 /* Push the arguments for an inferior function call, and return
@@ -317,7 +320,7 @@ amd64_windows_push_dummy_call
    function_call_return_method return_method, CORE_ADDR struct_addr)
 {
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  gdb_byte buf[8];
+  std::array<gdb_byte, 8> buf;
 
   /* Pass arguments.  */
   sp = amd64_windows_push_arguments (regcache, nargs, args, sp,
@@ -330,7 +333,7 @@ amd64_windows_push_dummy_call
         register.  */
       const int arg_regnum = amd64_windows_dummy_call_integer_regs[0];
 
-      store_unsigned_integer (buf, 8, byte_order, struct_addr);
+      store_unsigned_integer (buf, byte_order, struct_addr);
       regcache->cooked_write (arg_regnum, buf);
     }
 
@@ -340,11 +343,11 @@ amd64_windows_push_dummy_call
 
   /* Store return address.  */
   sp -= 8;
-  store_unsigned_integer (buf, 8, byte_order, bp_addr);
-  write_memory (sp, buf, 8);
+  store_unsigned_integer (buf, byte_order, bp_addr);
+  write_memory (sp, buf.data (), buf.size ());
 
   /* Update the stack pointer...  */
-  store_unsigned_integer (buf, 8, byte_order, sp);
+  store_unsigned_integer (buf, byte_order, sp);
   regcache->cooked_write (AMD64_RSP_REGNUM, buf);
 
   /* ...and fake a frame pointer.  */
@@ -433,13 +436,13 @@ amd64_skip_main_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
   target_read_memory (pc, &op, 1);
   if (op == 0xe8)
     {
-      gdb_byte buf[4];
+      std::array<gdb_byte, 4> buf;
 
-      if (target_read_memory (pc + 1, buf, sizeof buf) == 0)
+      if (target_read_memory (pc + 1, buf.data (), buf.size ()) == 0)
        {
          CORE_ADDR call_dest;
 
-         call_dest = pc + 5 + extract_signed_integer (buf, 4, byte_order);
+         call_dest = pc + 5 + extract_signed_integer (buf, byte_order);
          bound_minimal_symbol s = lookup_minimal_symbol_by_pc (call_dest);
          if (s.minsym != NULL
              && s.minsym->linkage_name () != NULL
@@ -617,12 +620,12 @@ amd64_windows_frame_decode_epilogue (const frame_info_ptr &this_frame,
     case 0xec:
       {
        /* jmp rel32  */
-       gdb_byte rel32[4];
+       std::array<gdb_byte, 4> rel32;
        CORE_ADDR npc;
 
-       if (target_read_memory (pc + 1, rel32, 4) != 0)
+       if (target_read_memory (pc + 1, rel32.data (), rel32.size ()) != 0)
          return -1;
-       npc = pc + 5 + extract_signed_integer (rel32, 4, byte_order);
+       npc = pc + 5 + extract_signed_integer (rel32, byte_order);
 
        /* If the jump is within the function, then this is not a marker,
           otherwise this is a tail-call.  */
@@ -632,13 +635,13 @@ amd64_windows_frame_decode_epilogue (const frame_info_ptr &this_frame,
     case 0xc2:
       {
        /* ret n  */
-       gdb_byte imm16[2];
+       std::array<gdb_byte, 2> imm16;
 
-       if (target_read_memory (pc + 1, imm16, 2) != 0)
+       if (target_read_memory (pc + 1, imm16.data (), imm16.size ()) != 0)
          return -1;
        cache->prev_rip_addr = cur_sp;
        cache->prev_sp = cur_sp
-         + extract_unsigned_integer (imm16, 4, byte_order);
+         + extract_unsigned_integer (imm16, byte_order);
        return 1;
       }
 
@@ -797,11 +800,11 @@ amd64_windows_frame_decode_insns (const frame_info_ptr &this_frame,
          /* According to msdn:
             If an FP reg is used, then any unwind code taking an offset must
             only be used after the FP reg is established in the prolog.  */
-         gdb_byte buf[8];
+         std::array<gdb_byte, 8> buf;
          int frreg = amd64_windows_w2gdb_regnum[frame_reg];
 
-         get_frame_register (this_frame, frreg, buf);
-         save_addr = extract_unsigned_integer (buf, 8, byte_order);
+         get_frame_register (this_frame, frreg, buf.data ());
+         save_addr = extract_unsigned_integer (buf, byte_order);
 
          frame_debug_printf ("   frame_reg=%s, val=%s",
                              gdbarch_register_name (gdbarch, frreg),
@@ -1084,7 +1087,7 @@ amd64_windows_frame_cache (const frame_info_ptr &this_frame, void **this_cache)
   struct gdbarch *gdbarch = get_frame_arch (this_frame);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
   struct amd64_windows_frame_cache *cache;
-  gdb_byte buf[8];
+  std::array<gdb_byte, 8> buf;
   CORE_ADDR pc;
   CORE_ADDR unwind_info = 0;
 
@@ -1096,8 +1099,8 @@ amd64_windows_frame_cache (const frame_info_ptr &this_frame, void **this_cache)
 
   /* Get current PC and SP.  */
   pc = get_frame_pc (this_frame);
-  get_frame_register (this_frame, AMD64_RSP_REGNUM, buf);
-  cache->sp = extract_unsigned_integer (buf, 8, byte_order);
+  get_frame_register (this_frame, AMD64_RSP_REGNUM, buf.data ());
+  cache->sp = extract_unsigned_integer (buf, byte_order);
   cache->pc = pc;
 
   /* If we can't find the unwind info, keep trying as though this is a