From db4ab410dec3554bf38f69879de2306ce5e25b7e Mon Sep 17 00:00:00 2001 From: Alan Modra Date: Tue, 5 Dec 2023 16:32:34 +1030 Subject: alpha_ecoff_get_relocated_section_contents buffer overflow This is aimed at fixing holes in two alpha-ecoff relocation functions that access section contents without first bounds checking offsets. I've also rewritten ALPHA_R_OP_STORE handling to support writing to the bytes near the end of the section. * coff-alpha.c (alpha_ecoff_get_relocated_section_contents): Don't bother checking ALPHA_R_LITERAL insn. Range check before reading contents for ALPHA_R_GPDISP, and simplify handling. Rewrite ALPHA_R_OP_STORE handling. Correct error callback args. (alpha_relocate_section): Similarly. Don't abort, report errors. --- bfd/coff-alpha.c | 480 ++++++++++++++++++++++++++++--------------------------- 1 file changed, 242 insertions(+), 238 deletions(-) diff --git a/bfd/coff-alpha.c b/bfd/coff-alpha.c index 884073a..3403e13 100644 --- a/bfd/coff-alpha.c +++ b/bfd/coff-alpha.c @@ -885,25 +885,15 @@ alpha_ecoff_get_relocated_section_contents (bfd *abfd, use. This would not be particularly difficult, but it is not currently implemented. */ - { - unsigned long insn; - - /* I believe that the LITERAL reloc will only apply to a - ldq or ldl instruction, so check my assumption. */ - insn = bfd_get_32 (input_bfd, data + rel->address); - BFD_ASSERT (((insn >> 26) & 0x3f) == 0x29 - || ((insn >> 26) & 0x3f) == 0x28); - - rel->addend -= gp; - r = bfd_perform_relocation (input_bfd, rel, data, input_section, - output_bfd, &err); - if (r == bfd_reloc_ok && gp_undefined) - { - r = bfd_reloc_dangerous; - err = - (char *) _("GP relative relocation used when GP not defined"); - } - } + rel->addend -= gp; + r = bfd_perform_relocation (input_bfd, rel, data, input_section, + output_bfd, &err); + if (r == bfd_reloc_ok && gp_undefined) + { + r = bfd_reloc_dangerous; + err = (char *) _("GP relative relocation used" + " when GP not defined"); + } break; case ALPHA_R_LITUSE: @@ -918,54 +908,46 @@ alpha_ecoff_get_relocated_section_contents (bfd *abfd, current location. The second of the pair is r_size bytes ahead; it used to be marked with an ALPHA_R_IGNORE reloc, but that no longer happens in OSF/1 3.2. */ - { - unsigned long insn1, insn2; - bfd_vma addend; - - /* Get the two instructions. */ - insn1 = bfd_get_32 (input_bfd, data + rel->address); - insn2 = bfd_get_32 (input_bfd, data + rel->address + rel->addend); - - BFD_ASSERT (((insn1 >> 26) & 0x3f) == 0x09); /* ldah */ - BFD_ASSERT (((insn2 >> 26) & 0x3f) == 0x08); /* lda */ + if (bfd_reloc_offset_in_range (rel->howto, input_bfd, input_section, + rel->address) + && bfd_reloc_offset_in_range (rel->howto, input_bfd, input_section, + rel->address + rel->addend)) + { + /* Get the two instructions. */ + bfd_byte *p = data + rel->address; + bfd_vma insn1 = bfd_get_32 (input_bfd, p); + bfd_vma insn2 = bfd_get_32 (input_bfd, p + rel->addend); + + BFD_ASSERT (((insn1 >> 26) & 0x3f) == 0x09); /* ldah */ + BFD_ASSERT (((insn2 >> 26) & 0x3f) == 0x08); /* lda */ + + /* Get the existing addend. We must account for the sign + extension done by lda and ldah. */ + bfd_vma addend = (((((insn1 & 0xffff) ^ 0x8000) - 0x8000) << 16) + + ((((insn2 & 0xffff) ^ 0x8000) - 0x8000))); + + /* The existing addend includes the different between the + gp of the input BFD and the address in the input BFD. + Subtract this out. */ + addend -= ecoff_data (input_bfd)->gp - input_section->vma; + + /* Now add in the final gp value, and subtract out the + final address. */ + addend += gp - (input_section->output_section->vma + + input_section->output_offset); + + /* Change the instructions, accounting for the sign + extension, and write them out. */ + insn1 = (insn1 & ~0xffff) | (((addend + 0x8000) >> 16) & 0xffff); + insn2 = (insn2 & ~0xffff) | (addend & 0xffff); + + bfd_put_32 (input_bfd, insn1, p); + bfd_put_32 (input_bfd, insn2, p + rel->addend); + } + else + r = bfd_reloc_outofrange; - /* Get the existing addend. We must account for the sign - extension done by lda and ldah. */ - addend = ((insn1 & 0xffff) << 16) + (insn2 & 0xffff); - if (insn1 & 0x8000) - { - addend -= 0x80000000; - addend -= 0x80000000; - } - if (insn2 & 0x8000) - addend -= 0x10000; - - /* The existing addend includes the different between the - gp of the input BFD and the address in the input BFD. - Subtract this out. */ - addend -= (ecoff_data (input_bfd)->gp - - (input_section->vma + rel->address)); - - /* Now add in the final gp value, and subtract out the - final address. */ - addend += (gp - - (input_section->output_section->vma - + input_section->output_offset - + rel->address)); - - /* Change the instructions, accounting for the sign - extension, and write them out. */ - if (addend & 0x8000) - addend += 0x10000; - insn1 = (insn1 & 0xffff0000) | ((addend >> 16) & 0xffff); - insn2 = (insn2 & 0xffff0000) | (addend & 0xffff); - - bfd_put_32 (input_bfd, (bfd_vma) insn1, data + rel->address); - bfd_put_32 (input_bfd, (bfd_vma) insn2, - data + rel->address + rel->addend); - - rel->address += input_section->output_offset; - } + rel->address += input_section->output_offset; break; case ALPHA_R_OP_PUSH: @@ -1007,9 +989,6 @@ alpha_ecoff_get_relocated_section_contents (bfd *abfd, case ALPHA_R_OP_STORE: /* Store a value from the reloc stack into a bitfield. */ { - bfd_vma val; - int offset, size; - if (relocatable) { rel->address += input_section->output_offset; @@ -1022,15 +1001,36 @@ alpha_ecoff_get_relocated_section_contents (bfd *abfd, break; } - /* The offset and size for this reloc are encoded into the - addend field by alpha_adjust_reloc_in. */ - offset = (rel->addend >> 8) & 0xff; - size = rel->addend & 0xff; + /* The offset and size in bits for this reloc are encoded + into the addend field by alpha_adjust_reloc_in. */ + unsigned int offset = (rel->addend >> 8) & 0xff; + unsigned int size = rel->addend & 0xff; + unsigned int startbyte = offset >> 3; + unsigned int endbyte = (offset + size + 7) >> 3; + unsigned int bytes = endbyte + 1 - startbyte; + + if (bytes <= 8 + && rel->address + startbyte + bytes >= rel->address + && (rel->address + startbyte + bytes + <= bfd_get_section_limit_octets (input_bfd, input_section))) + { + uint64_t val = 0; + for (int off = bytes - 1; off >= 0; --off) + val = (val << 8) | data[rel->address + startbyte + off]; + + offset -= startbyte << 3; + size -= startbyte << 3; + uint64_t mask = (((uint64_t) 1 << size) - 1) << offset; + val = (val & ~mask) | ((stack[--tos] << offset) & mask); - val = bfd_get_64 (abfd, data + rel->address); - val &=~ (((1 << size) - 1) << offset); - val |= (stack[--tos] & ((1 << size) - 1)) << offset; - bfd_put_64 (abfd, val, data + rel->address); + for (unsigned int off = 0; off < bytes; ++off) + { + data[rel->address + startbyte + off] = val & 0xff; + val >>= 8; + } + } + else + r = bfd_reloc_outofrange; } break; @@ -1149,20 +1149,20 @@ alpha_ecoff_get_relocated_section_contents (bfd *abfd, (*link_info->callbacks->einfo) /* xgettext:c-format */ (_("%X%P: %pB(%pA): relocation \"%pR\" goes out of range\n"), - abfd, input_section, rel); + input_bfd, input_section, rel); goto error_return; case bfd_reloc_notsupported: (*link_info->callbacks->einfo) /* xgettext:c-format */ (_("%X%P: %pB(%pA): relocation \"%pR\" is not supported\n"), - abfd, input_section, rel); + input_bfd, input_section, rel); goto error_return; default: (*link_info->callbacks->einfo) /* xgettext:c-format */ (_("%X%P: %pB(%pA): relocation \"%pR\"" " returns an unrecognized value %x\n"), - abfd, input_section, rel, r); + input_bfd, input_section, rel, r); break; } } @@ -1389,6 +1389,7 @@ alpha_relocate_section (bfd *output_bfd, struct external_reloc *ext_rel; struct external_reloc *ext_rel_end; bfd_size_type amt; + bool ret = true; /* We keep a table mapping the symndx found in an internal reloc to the appropriate section. This is faster than looking up the @@ -1522,6 +1523,7 @@ alpha_relocate_section (bfd *output_bfd, bool adjust_addrp; bool gp_usedp; bfd_vma addend; + bfd_reloc_status_type r; r_vaddr = H_GET_64 (input_bfd, ext_rel->r_vaddr); r_symndx = H_GET_32 (input_bfd, ext_rel->r_symndx); @@ -1539,27 +1541,13 @@ alpha_relocate_section (bfd *output_bfd, adjust_addrp = true; gp_usedp = false; addend = 0; + r = bfd_reloc_ok; switch (r_type) { - case ALPHA_R_GPRELHIGH: - _bfd_error_handler (_("%pB: %s unsupported"), - input_bfd, "ALPHA_R_GPRELHIGH"); - bfd_set_error (bfd_error_bad_value); - continue; - - case ALPHA_R_GPRELLOW: - _bfd_error_handler (_("%pB: %s unsupported"), - input_bfd, "ALPHA_R_GPRELLOW"); - bfd_set_error (bfd_error_bad_value); - continue; - default: - /* xgettext:c-format */ - _bfd_error_handler (_("%pB: unsupported relocation type %#x"), - input_bfd, (int) r_type); - bfd_set_error (bfd_error_bad_value); - continue; + r = bfd_reloc_notsupported; + break; case ALPHA_R_IGNORE: /* This reloc appears after a GPDISP reloc. On earlier @@ -1616,17 +1604,6 @@ alpha_relocate_section (bfd *output_bfd, use. This would not be particularly difficult, but it is not currently implemented. */ - /* I believe that the LITERAL reloc will only apply to a ldq - or ldl instruction, so check my assumption. */ - { - unsigned long insn; - - insn = bfd_get_32 (input_bfd, - contents + r_vaddr - input_section->vma); - BFD_ASSERT (((insn >> 26) & 0x3f) == 0x29 - || ((insn >> 26) & 0x3f) == 0x28); - } - relocatep = true; addend = ecoff_data (input_bfd)->gp - gp; gp_usedp = true; @@ -1643,58 +1620,45 @@ alpha_relocate_section (bfd *output_bfd, current location. The second of the pair is r_symndx bytes ahead. It used to be marked with an ALPHA_R_IGNORE reloc, but OSF/1 3.2 no longer does that. */ - { - unsigned long insn1, insn2; - - /* Get the two instructions. */ - insn1 = bfd_get_32 (input_bfd, - contents + r_vaddr - input_section->vma); - insn2 = bfd_get_32 (input_bfd, - (contents - + r_vaddr - - input_section->vma - + r_symndx)); - - BFD_ASSERT (((insn1 >> 26) & 0x3f) == 0x09); /* ldah */ - BFD_ASSERT (((insn2 >> 26) & 0x3f) == 0x08); /* lda */ - - /* Get the existing addend. We must account for the sign - extension done by lda and ldah. */ - addend = ((insn1 & 0xffff) << 16) + (insn2 & 0xffff); - if (insn1 & 0x8000) - { - /* This is addend -= 0x100000000 without causing an - integer overflow on a 32 bit host. */ - addend -= 0x80000000; - addend -= 0x80000000; - } - if (insn2 & 0x8000) - addend -= 0x10000; - - /* The existing addend includes the difference between the - gp of the input BFD and the address in the input BFD. - We want to change this to the difference between the - final GP and the final address. */ - addend += (gp - - ecoff_data (input_bfd)->gp - + input_section->vma - - (input_section->output_section->vma - + input_section->output_offset)); - - /* Change the instructions, accounting for the sign - extension, and write them out. */ - if (addend & 0x8000) - addend += 0x10000; - insn1 = (insn1 & 0xffff0000) | ((addend >> 16) & 0xffff); - insn2 = (insn2 & 0xffff0000) | (addend & 0xffff); - - bfd_put_32 (input_bfd, (bfd_vma) insn1, - contents + r_vaddr - input_section->vma); - bfd_put_32 (input_bfd, (bfd_vma) insn2, - contents + r_vaddr - input_section->vma + r_symndx); - - gp_usedp = true; - } + if (r_vaddr >= input_section->vma + && r_vaddr - input_section->vma < input_section->size + && input_section->size - (r_vaddr - input_section->vma) > r_symndx + && (input_section->size - (r_vaddr - input_section->vma) + - r_symndx >= 4)) + { + /* Get the two instructions. */ + bfd_byte *p = contents + r_vaddr - input_section->vma; + bfd_vma insn1 = bfd_get_32 (input_bfd, p); + bfd_vma insn2 = bfd_get_32 (input_bfd, p + r_symndx); + + BFD_ASSERT (((insn1 >> 26) & 0x3f) == 0x09); /* ldah */ + BFD_ASSERT (((insn2 >> 26) & 0x3f) == 0x08); /* lda */ + + /* Get the existing addend. We must account for the sign + extension done by lda and ldah. */ + addend = (((((insn1 & 0xffff) ^ 0x8000) - 0x8000) << 16) + + (((insn2 & 0xffff) ^ 0x8000) - 0x8000)); + + /* The existing addend includes the difference between the + gp of the input BFD and the address in the input BFD. + We want to change this to the difference between the + final GP and the final address. */ + addend -= ecoff_data (input_bfd)->gp - input_section->vma; + addend += gp - (input_section->output_section->vma + + input_section->output_offset); + + /* Change the instructions, accounting for the sign + extension, and write them out. */ + insn1 = (insn1 & ~0xffff) | (((addend + 0x8000) >> 16) & 0xffff); + insn2 = (insn2 & ~0xffff) | (addend & 0xffff); + + bfd_put_32 (input_bfd, insn1, p); + bfd_put_32 (input_bfd, insn2, p + r_symndx); + + gp_usedp = true; + } + else + r = bfd_reloc_outofrange; break; case ALPHA_R_OP_PUSH: @@ -1709,8 +1673,11 @@ alpha_relocate_section (bfd *output_bfd, asection *s; s = symndx_to_section[r_symndx]; - if (s == (asection *) NULL) - abort (); + if (s == NULL) + { + r = bfd_reloc_notsupported; + break; + } addend = s->output_section->vma + s->output_offset - s->vma; } else @@ -1718,8 +1685,11 @@ alpha_relocate_section (bfd *output_bfd, struct ecoff_link_hash_entry *h; h = sym_hashes[r_symndx]; - if (h == (struct ecoff_link_hash_entry *) NULL) - abort (); + if (h == NULL) + { + r = bfd_reloc_notsupported; + break; + } if (! bfd_link_relocatable (info)) { @@ -1773,19 +1743,28 @@ alpha_relocate_section (bfd *output_bfd, { case ALPHA_R_OP_PUSH: if (tos >= RELOC_STACKSIZE) - abort (); + { + r = bfd_reloc_notsupported; + break; + } stack[tos++] = addend; break; case ALPHA_R_OP_PSUB: if (tos == 0) - abort (); + { + r = bfd_reloc_notsupported; + break; + } stack[tos - 1] -= addend; break; case ALPHA_R_OP_PRSHIFT: if (tos == 0) - abort (); + { + r = bfd_reloc_notsupported; + break; + } stack[tos - 1] >>= addend; break; } @@ -1800,28 +1779,34 @@ alpha_relocate_section (bfd *output_bfd, adjust the address of the reloc. */ if (! bfd_link_relocatable (info)) { - bfd_vma mask; - bfd_vma val; - - if (tos == 0) - abort (); - - /* Get the relocation mask. The separate steps and the - casts to bfd_vma are attempts to avoid a bug in the - Alpha OSF 1.3 C compiler. See reloc.c for more - details. */ - mask = 1; - mask <<= (bfd_vma) r_size; - mask -= 1; - - /* FIXME: I don't know what kind of overflow checking, - if any, should be done here. */ - val = bfd_get_64 (input_bfd, - contents + r_vaddr - input_section->vma); - val &=~ mask << (bfd_vma) r_offset; - val |= (stack[--tos] & mask) << (bfd_vma) r_offset; - bfd_put_64 (input_bfd, val, - contents + r_vaddr - input_section->vma); + unsigned int startbyte = r_offset >> 3; + unsigned int endbyte = (r_offset + r_size + 7) >> 3; + unsigned int bytes = endbyte + 1 - startbyte; + + if (bytes <= 8 + && r_vaddr >= input_section->vma + && r_vaddr - input_section->vma < input_section->size + && (input_section->size - (r_vaddr - input_section->vma) + >= startbyte + bytes)) + { + bfd_byte *p = contents + (r_vaddr - input_section->vma); + uint64_t val = 0; + for (int off = bytes - 1; off >= 0; --off) + val = (val << 8) | p[startbyte + off]; + + r_offset -= startbyte << 3; + r_size -= startbyte << 3; + uint64_t mask = (((uint64_t) 1 << r_size) - 1) << r_offset; + val = (val & ~mask) | ((stack[--tos] << r_offset) & mask); + + for (unsigned int off = 0; off < bytes; ++off) + { + p[startbyte + off] = val & 0xff; + val >>= 8; + } + } + else + r = bfd_reloc_outofrange; } break; @@ -1832,13 +1817,12 @@ alpha_relocate_section (bfd *output_bfd, break; } - if (relocatep) + if (relocatep && r == bfd_reloc_ok) { reloc_howto_type *howto; struct ecoff_link_hash_entry *h = NULL; asection *s = NULL; bfd_vma relocation; - bfd_reloc_status_type r; /* Perform a relocation. */ @@ -1850,8 +1834,8 @@ alpha_relocate_section (bfd *output_bfd, /* If h is NULL, that means that there is a reloc against an external symbol which we thought was just a debugging symbol. This should not happen. */ - if (h == (struct ecoff_link_hash_entry *) NULL) - abort (); + if (h == NULL) + r = bfd_reloc_notsupported; } else { @@ -1860,11 +1844,14 @@ alpha_relocate_section (bfd *output_bfd, else s = symndx_to_section[r_symndx]; - if (s == (asection *) NULL) - abort (); + if (s == NULL) + r = bfd_reloc_notsupported; + } - if (bfd_link_relocatable (info)) + if (r != bfd_reloc_ok) + ; + else if (bfd_link_relocatable (info)) { /* We are generating relocatable output, and must convert the existing reloc. */ @@ -1930,12 +1917,7 @@ alpha_relocate_section (bfd *output_bfd, + hsec->output_offset); } else - { - (*info->callbacks->undefined_symbol) - (info, h->root.root.string, input_bfd, input_section, - r_vaddr - input_section->vma, true); - relocation = 0; - } + r = bfd_reloc_undefined; } else { @@ -1950,37 +1932,14 @@ alpha_relocate_section (bfd *output_bfd, relocation += input_section->vma; } - r = _bfd_final_link_relocate (howto, - input_bfd, - input_section, - contents, - r_vaddr - input_section->vma, - relocation, - addend); - } - - if (r != bfd_reloc_ok) - { - switch (r) - { - default: - case bfd_reloc_outofrange: - abort (); - case bfd_reloc_overflow: - { - const char *name; - - if (r_extern) - name = sym_hashes[r_symndx]->root.root.string; - else - name = bfd_section_name (symndx_to_section[r_symndx]); - (*info->callbacks->reloc_overflow) - (info, NULL, name, alpha_howto_table[r_type].name, - (bfd_vma) 0, input_bfd, input_section, - r_vaddr - input_section->vma); - } - break; - } + if (r == bfd_reloc_ok) + r = _bfd_final_link_relocate (howto, + input_bfd, + input_section, + contents, + r_vaddr - input_section->vma, + relocation, + addend); } } @@ -1997,20 +1956,65 @@ alpha_relocate_section (bfd *output_bfd, if (gp_usedp && gp_undefined) { - (*info->callbacks->reloc_dangerous) - (info, _("GP relative relocation used when GP not defined"), - input_bfd, input_section, r_vaddr - input_section->vma); + r = bfd_reloc_dangerous; /* Only give the error once per link. */ gp = 4; _bfd_set_gp_value (output_bfd, gp); gp_undefined = false; } + + if (r != bfd_reloc_ok) + { + switch (r) + { + case bfd_reloc_overflow: + { + const char *name; + + if (r_extern) + name = sym_hashes[r_symndx]->root.root.string; + else + name = bfd_section_name (symndx_to_section[r_symndx]); + (*info->callbacks->reloc_overflow) + (info, NULL, name, alpha_howto_table[r_type].name, + (bfd_vma) 0, input_bfd, input_section, + r_vaddr - input_section->vma); + } + break; + case bfd_reloc_outofrange: + (*info->callbacks->einfo) + /* xgettext:c-format */ + (_("%X%P: %pB(%pA): relocation out of range\n"), + input_bfd, input_section); + break; + case bfd_reloc_undefined: + (*info->callbacks->undefined_symbol) + (info, sym_hashes[r_symndx]->root.root.string, + input_bfd, input_section, + r_vaddr - input_section->vma, true); + break; + case bfd_reloc_notsupported: + (*info->callbacks->einfo) + /* xgettext:c-format */ + (_("%X%P: %pB(%pA): relocation is not supported\n"), + input_bfd, input_section); + break; + case bfd_reloc_dangerous: + (*info->callbacks->reloc_dangerous) + (info, _("GP relative relocation used when GP not defined"), + input_bfd, input_section, r_vaddr - input_section->vma); + break; + default: + abort (); + } + ret = false; + } } if (tos != 0) - abort (); + ret = false; - return true; + return ret; } /* Do final adjustments to the filehdr and the aouthdr. This routine -- cgit v1.1