aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLionel Flandrin <lionel@svkt.org>2016-11-08 10:27:36 +0000
committerPedro Alves <palves@redhat.com>2016-11-08 10:27:36 +0000
commit9dc193c3be85aafa60ceff57d3b0430af607b4ce (patch)
treed900edb55d55df5315db1db846c6ac9dd16fde49
parentec7b600bf1410f6bda239666fac258a605dc3f43 (diff)
downloadgdb-9dc193c3be85aafa60ceff57d3b0430af607b4ce.zip
gdb-9dc193c3be85aafa60ceff57d3b0430af607b4ce.tar.gz
gdb-9dc193c3be85aafa60ceff57d3b0430af607b4ce.tar.bz2
Check for truncated registers in process_g_packet
While investigating an unrelated issue in remote.c I noticed that the bound checking for 'g' packets was bogus: The previous code would only check that the first byte of the register was within bounds before passing the buffer to regcache_raw_supply. If it turned out that the register in the 'g' packet was incomplete then regcache_raw_supply would proceed to memcpy out-of-bounds. Since the buffer is allocated with alloca it's relatively unlikely to crash (you just end up dumping gdb's stack into the cache) but it's still a bit messy. I changed this logic to check for truncated registers and raise an error if one is encountered. Hopefully it should make debugging remote stubs a bit easier. gdb/ChangeLog: 2016-11-08 Lionel Flandrin <lionel@svkt.org> * remote.c (process_g_packet): Detect truncated registers in 'g' packets and raise an error.
-rw-r--r--gdb/ChangeLog5
-rw-r--r--gdb/remote.c20
2 files changed, 22 insertions, 3 deletions
diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index fbdb168..f029938 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2016-11-08 Lionel Flandrin <lionel@svkt.org>
+
+ * remote.c (process_g_packet): Detect truncated registers in 'g'
+ packets and raise an error.
+
2016-11-07 Doug Evans <dje@google.com>
* guile/scm-value.c (gdbscm_value_field): Fix call to value_struct_elt.
diff --git a/gdb/remote.c b/gdb/remote.c
index cf960e5..6c40f02 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -7579,18 +7579,31 @@ process_g_packet (struct regcache *regcache)
the 'p' packet must be used. */
if (buf_len < 2 * rsa->sizeof_g_packet)
{
- rsa->sizeof_g_packet = buf_len / 2;
+ long sizeof_g_packet = buf_len / 2;
for (i = 0; i < gdbarch_num_regs (gdbarch); i++)
{
+ long offset = rsa->regs[i].offset;
+ long reg_size = register_size (gdbarch, i);
+
if (rsa->regs[i].pnum == -1)
continue;
- if (rsa->regs[i].offset >= rsa->sizeof_g_packet)
+ if (offset >= sizeof_g_packet)
rsa->regs[i].in_g_packet = 0;
+ else if (offset + reg_size > sizeof_g_packet)
+ error (_("Truncated register %d in remote 'g' packet"), i);
else
rsa->regs[i].in_g_packet = 1;
}
+
+ /* Looks valid enough, we can assume this is the correct length
+ for a 'g' packet. It's important not to adjust
+ rsa->sizeof_g_packet if we have truncated registers otherwise
+ this "if" won't be run the next time the method is called
+ with a packet of the same size and one of the internal errors
+ below will trigger instead. */
+ rsa->sizeof_g_packet = sizeof_g_packet;
}
regs = (char *) alloca (rsa->sizeof_g_packet);
@@ -7620,10 +7633,11 @@ process_g_packet (struct regcache *regcache)
for (i = 0; i < gdbarch_num_regs (gdbarch); i++)
{
struct packet_reg *r = &rsa->regs[i];
+ long reg_size = register_size (gdbarch, i);
if (r->in_g_packet)
{
- if (r->offset * 2 >= strlen (rs->buf))
+ if ((r->offset + reg_size) * 2 > strlen (rs->buf))
/* This shouldn't happen - we adjusted in_g_packet above. */
internal_error (__FILE__, __LINE__,
_("unexpected end of 'g' packet reply"));