]> git.ipfire.org Git - thirdparty/binutils-gdb.git/commit
gdb: return when exceeding buffer size in regcache::transfer_regset
authorSimon Marchi <simon.marchi@efficios.com>
Tue, 28 Nov 2023 19:48:57 +0000 (14:48 -0500)
committerSimon Marchi <simon.marchi@efficios.com>
Fri, 1 Dec 2023 16:20:09 +0000 (11:20 -0500)
commitc47886c7a1dd66d8b8ada57f53685d238fac2510
tree0332c5063843922f3f1fbeb4e485cee31b7e8092
parentf644dde103cc34ce5fca7ec1d754877ec5d3b5c8
gdb: return when exceeding buffer size in regcache::transfer_regset

regcache::transfer_regset iterates over an array of regcache_map_entry,
transferring the registers (between regcache and buffer) described by
those entries.  It stops either when it reaches the end of the
regcache_map_entry array (marked by a null entry) or (it seems like the
intent is) when it reaches the end of the buffer (in which case not all
described registers are transferred).

I said "seems like the intent is", because there appears to be a small
bug.  transfer_regset is made of two loops:

    foreach regcache_map_entry:
      foreach register described by the regcache_map_entry:
        if the register doesn't fit in the remainder of the buffer:
  break

        transfer register

When stopping because we have reached the end of the buffer, the break
only breaks out of the inner loop.

This problem causes some failures when I run tests such as
gdb.arch/aarch64-sme-core-3.exp (on AArch64 Linux, in qemu).  This is
partly due to aarch64_linux_iterate_over_regset_sections failing to add
a null terminator in its regcache_map_entry array, but I think there is
still a problem in transfer_regset.

The sequence to the crash is:

 - The `regcache_map_entry za_regmap` object built in
   aarch64_linux_iterate_over_regset_sections does not have a null
   terminator.
 - When the target does not have a ZA register,
   aarch64_linux_collect_za_regset calls `regcache->collect_regset` with
   a size of 0 (it's actually pointless, but still it should work).
 - transfer_regset gets called with a buffer size of 0.
 - transfer_regset detects that the register to transfer wouldn't fit in
   0 bytes, so it breaks out of the inner loop.
 - The outer loop tries to go read the next regcache_map_entry, but
   there isn't one, and we start reading garbage.

Obviously, this would get fixed by making
aarch64_linux_iterate_over_regset_sections use a null terminator (which
is what the following patch does).  But I think that when detecting that
there is not enough buffer left for the current register,
transfer_regset should return, not only break out of the inner loop.

This is a kind of contrived scenario, but imagine we have these two
regcache_map_entry objects:

  - 2 registers of 8 bytes
  - 2 registers of 4 bytes

For some reason, the caller passes a buffer of 12 bytes.
transfer_regset will detect that the second 8 byte register does not
fit, and break out of the inner loop.  However, it will then go try the
next regcache_map_entry.  It will see that it can fit one 4 byte
register in the remaining buffer space, and transfer it from/to there.
This is very likely not an expected behavior, we wouldn't expect to
read/write this sequence of registers from/to the buffer.

In this example, whether passing a 12 bytes buffer makes sense or
whether it is a size computation bug in the caller, we don't know, but I
think that exiting as soon as a register doesn't fit is the sane thing
to do.

Change-Id: Ia349627d2e5d281822ade92a8e7a4dea4f839e07
Reviewed-By: John Baldwin <jhb@FreeBSD.org>
Reviewed-By: Luis Machado <luis.machado@arm.com>
gdb/regcache.c