diff options
author | Tom de Vries <tdevries@suse.de> | 2024-10-30 13:30:51 +0100 |
---|---|---|
committer | Tom de Vries <tdevries@suse.de> | 2024-10-30 13:30:51 +0100 |
commit | 35d53ce6429a5e822aff29803956eb008775ef15 (patch) | |
tree | 7e6dd2bdc05a645e412d0066c1f9e1eacee1cf68 | |
parent | 5330d85af1dbf48156a35f3908571ba57aae5304 (diff) | |
download | binutils-35d53ce6429a5e822aff29803956eb008775ef15.zip binutils-35d53ce6429a5e822aff29803956eb008775ef15.tar.gz binutils-35d53ce6429a5e822aff29803956eb008775ef15.tar.bz2 |
[gdb/tdep] Use std::array in amd64-windows-tdep.c
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>
-rw-r--r-- | gdb/amd64-windows-tdep.c | 51 |
1 files changed, 27 insertions, 24 deletions
diff --git a/gdb/amd64-windows-tdep.c b/gdb/amd64-windows-tdep.c index 57dcc4f..df59a44 100644 --- a/gdb/amd64-windows-tdep.c +++ b/gdb/amd64-windows-tdep.c @@ -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 |