[binutils-gdb] buffer_read_memory hardening
Alan Modra via Binutils-cvs <[email protected]> Wed, 17 Jun 2026 23:56:05 +0000 (GMT)
| Newsgroups | gmane.comp.gnu.binutils.cvs |
|---|---|
| Message-ID | <[email protected]> |
https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=b81d60ecd1b5ffadd80f51d0d387de3762f47b46 commit b81d60ecd1b5ffadd80f51d0d387de3762f47b46 Author: Alan Modra <[email protected]> Date: Thu Jun 18 09:22:32 2026 +0930 buffer_read_memory hardening What drew my attention here was wondering if division of the buffer length or the requested length by octets_per_byte could result in trucation and thus an incomplete check for possible buffer overflow. Rather than convincing myself that this couldn't happen, I decided to rewrite the checks in a way that avoids the possibility of truncation, and avoids any addition overflows too. * dis-buf.c (buffer_read_memory): Rewrite buffer overflow sanity checks. Diff: --- opcodes/dis-buf.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/opcodes/dis-buf.c b/opcodes/dis-buf.c index 83fce0a2fc3..504d0b37b98 100644 --- a/opcodes/dis-buf.c +++ b/opcodes/dis-buf.c @@ -32,18 +32,19 @@ buffer_read_memory (bfd_vma memaddr, struct disassemble_info *info) { unsigned int opb = info->octets_per_byte; - size_t end_addr_offset = length / opb; - size_t max_addr_offset = info->buffer_length / opb; - size_t octets = (memaddr - info->buffer_vma) * opb; + size_t addr_off, to_stop; if (memaddr < info->buffer_vma - || memaddr - info->buffer_vma > max_addr_offset - || memaddr - info->buffer_vma + end_addr_offset > max_addr_offset - || (info->stop_vma && (memaddr >= info->stop_vma - || memaddr + end_addr_offset > info->stop_vma))) + || _bfd_mul_overflow (memaddr - info->buffer_vma, opb, &addr_off) + || addr_off > info->buffer_length + || length > info->buffer_length - addr_off + || (info->stop_vma + && (memaddr >= info->stop_vma + || _bfd_mul_overflow (info->stop_vma - memaddr, opb, &to_stop) + || length > to_stop))) /* Out of bounds. Use EIO because GDB uses it. */ return EIO; - memcpy (myaddr, info->buffer + octets, length); + memcpy (myaddr, info->buffer + addr_off, length); return 0; }