diff options
author | Lionel Flandrin <lionel@svkt.org> | 2016-11-08 10:27:36 +0000 |
---|---|---|
committer | Pedro Alves <palves@redhat.com> | 2016-11-08 10:27:36 +0000 |
commit | 9dc193c3be85aafa60ceff57d3b0430af607b4ce (patch) | |
tree | d900edb55d55df5315db1db846c6ac9dd16fde49 | |
parent | ec7b600bf1410f6bda239666fac258a605dc3f43 (diff) | |
download | gdb-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/ChangeLog | 5 | ||||
-rw-r--r-- | gdb/remote.c | 20 |
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")); |