From: Simon Marchi Date: Tue, 28 Nov 2023 19:48:57 +0000 (-0500) Subject: gdb: return when exceeding buffer size in regcache::transfer_regset X-Git-Tag: binutils-2_42~749 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=c47886c7a1dd66d8b8ada57f53685d238fac2510;p=thirdparty%2Fbinutils-gdb.git 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 Reviewed-By: Luis Machado --- diff --git a/gdb/regcache.c b/gdb/regcache.c index 9dc354ec2b3..e46a0b58f50 100644 --- a/gdb/regcache.c +++ b/gdb/regcache.c @@ -1208,7 +1208,7 @@ regcache::transfer_regset (const struct regset *regset, int regbase, for (; count--; regno++, offs += slot_size) { if (offs + slot_size > size) - break; + return; transfer_regset_register (out_regcache, regno, in_buf, out_buf, slot_size, offs);