inconsistencies if (at least) ELF relocation handling
Jan Beulich
jbeulich@suse.com
Mon Apr 19 09:05:19 GMT 2021
All,
while further learning how exactly bfd processes relocations, I
came to notice that even ELF RELA relocations have section contents
read and whatever was found added into the new value to be stored
(which I take to mean that there is an assumption that these fields
are emitted as all zero by whichever tool creates the object files,
which even with gas can be easily violated using the .reloc pseudo).
The comment next to struct reloc_howto_struct's partial_inplace field
suggests that this is to be avoided by setting src_mask to zero. I
can see that e.g. PPC, IA64, and RISC-V do so, but e.g. x86-64, Arm32,
and Arm64 don't (I've looked at just the architectures that I'm at
least remotely familiar with). Any thoughts about fixing this (Cc-ing
the specific targets' maintainers for this reason)?
However, even if src_mask was zero for all targets using only RELA
relocations, there's then still an asymmetry in overflow checking:
REL relocs, having their addends read out of section contents by e.g.
_bfd_relocate_contents(), have overflow from adding in this addend
properly checked. RELA relocs, having their addends added in already
in e.g. _bfd_final_link_relocate(), don't. I've started putting
together a patch (appended at the end of the mail), which is both
incomplete (neither adjusting target-specific callers of
_bfd_relocate_contents() yet, nor adjusting further callers of
read_reloc() so far) and breaking at least x86-64 (see below). I'd
still like to get input on what people think a proper approach would
be here.
As to the breakage on x86-64, I observe a number of "relocation
truncated to fit: R_X86_64_32 against `.debug_str'", due to negative
addends getting truncated to 32-bit unsigned values and this reloc
using complain_overflow_unsigned. In ELF it is my understanding that
addends - no matter what their origin - are always signed. For this
case the present checking done for complain_overflow_unsigned looks
wrong to me. But of course there may be object formats where unsigned
addends might be used for at least some relocation types, so I'm
hesitant to change the checking logic itself, and I'm instead
wondering whether yet another complain_overflow_* type may be needed.
Otoh there are only very few uses of the type outside of bfd/elf*.c.
Jan
--- a/bfd/cofflink.c
+++ b/bfd/cofflink.c
@@ -2822,7 +2822,7 @@ _bfd_coff_reloc_link_order (bfd *output_
if (buf == NULL && size != 0)
return FALSE;
- rstat = _bfd_relocate_contents (howto, output_bfd,
+ rstat = _bfd_relocate_contents (howto, output_bfd, 0,
(bfd_vma) link_order->u.reloc.p->addend,
buf);
switch (rstat)
--- a/bfd/ecoff.c
+++ b/bfd/ecoff.c
@@ -3953,7 +3953,7 @@ ecoff_reloc_link_order (bfd *output_bfd,
buf = (bfd_byte *) bfd_zmalloc (size);
if (buf == NULL && size != 0)
return FALSE;
- rstat = _bfd_relocate_contents (rel.howto, output_bfd,
+ rstat = _bfd_relocate_contents (rel.howto, output_bfd, 0,
(bfd_vma) addend, buf);
switch (rstat)
{
--- a/bfd/elf32-i386.c
+++ b/bfd/elf32-i386.c
@@ -130,7 +130,7 @@ static reloc_howto_type elf_howto_table[
HOWTO(R_386_TLS_GOTDESC, 0, 2, 32, FALSE, 0, complain_overflow_dont,
bfd_elf_generic_reloc, "R_386_TLS_GOTDESC",
TRUE, 0xffffffff, 0xffffffff, FALSE),
- HOWTO(R_386_TLS_DESC_CALL, 0, 0, 0, FALSE, 0, complain_overflow_dont,
+ HOWTO(R_386_TLS_DESC_CALL, 0, 3, 0, FALSE, 0, complain_overflow_dont,
bfd_elf_generic_reloc, "R_386_TLS_DESC_CALL",
FALSE, 0, 0, FALSE),
HOWTO(R_386_TLS_DESC, 0, 2, 32, FALSE, 0, complain_overflow_dont,
--- a/bfd/elf64-x86-64.c
+++ b/bfd/elf64-x86-64.c
@@ -146,7 +146,7 @@ static reloc_howto_type x86_64_elf_howto
complain_overflow_bitfield, bfd_elf_generic_reloc,
"R_X86_64_GOTPC32_TLSDESC",
FALSE, 0xffffffff, 0xffffffff, TRUE),
- HOWTO(R_X86_64_TLSDESC_CALL, 0, 0, 0, FALSE, 0,
+ HOWTO(R_X86_64_TLSDESC_CALL, 0, 3, 0, FALSE, 0,
complain_overflow_dont, bfd_elf_generic_reloc,
"R_X86_64_TLSDESC_CALL",
FALSE, 0, 0, FALSE),
--- a/bfd/elflink.c
+++ b/bfd/elflink.c
@@ -11839,7 +11839,7 @@ elf_reloc_link_order (bfd *output_bfd,
buf = (bfd_byte *) bfd_zmalloc (size);
if (buf == NULL && size != 0)
return FALSE;
- rstat = _bfd_relocate_contents (howto, output_bfd, addend, buf);
+ rstat = _bfd_relocate_contents (howto, output_bfd, 0, addend, buf);
switch (rstat)
{
case bfd_reloc_ok:
--- a/bfd/libbfd-in.h
+++ b/bfd/libbfd-in.h
@@ -696,7 +696,8 @@ extern bfd_reloc_status_type _bfd_final_
/* Relocate a particular location by a howto and a value. */
extern bfd_reloc_status_type _bfd_relocate_contents
- (reloc_howto_type *, bfd *, bfd_vma, bfd_byte *) ATTRIBUTE_HIDDEN;
+ (reloc_howto_type *, bfd *, bfd_vma, bfd_signed_vma, bfd_byte *)
+ ATTRIBUTE_HIDDEN;
/* Clear a given location using a given howto. */
extern bfd_reloc_status_type _bfd_clear_contents
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -701,7 +701,8 @@ extern bfd_reloc_status_type _bfd_final_
/* Relocate a particular location by a howto and a value. */
extern bfd_reloc_status_type _bfd_relocate_contents
- (reloc_howto_type *, bfd *, bfd_vma, bfd_byte *) ATTRIBUTE_HIDDEN;
+ (reloc_howto_type *, bfd *, bfd_vma, bfd_signed_vma, bfd_byte *)
+ ATTRIBUTE_HIDDEN;
/* Clear a given location using a given howto. */
extern bfd_reloc_status_type _bfd_clear_contents
--- a/bfd/linker.c
+++ b/bfd/linker.c
@@ -2393,7 +2393,7 @@ _bfd_generic_reloc_link_order (bfd *abfd
buf = (bfd_byte *) bfd_zmalloc (size);
if (buf == NULL && size != 0)
return FALSE;
- rstat = _bfd_relocate_contents (r->howto, abfd,
+ rstat = _bfd_relocate_contents (r->howto, abfd, 0,
(bfd_vma) link_order->u.reloc.p->addend,
buf);
switch (rstat)
--- a/bfd/reloc.c
+++ b/bfd/reloc.c
@@ -1355,7 +1355,12 @@ _bfd_final_link_relocate (reloc_howto_ty
bfd_vma value,
bfd_vma addend)
{
- bfd_vma relocation;
+ /* This function assumes that we are dealing with a basic relocation
+ against a symbol. We want to compute the value of the symbol to
+ relocate to. This is just VALUE, the value of the symbol, plus
+ ADDEND, any addend associated with the reloc (which will get added
+ in by _bfd_relocate_contents()). */
+ bfd_vma relocation = value;
bfd_size_type octets = (address
* bfd_octets_per_byte (input_bfd, input_section));
@@ -1363,12 +1368,6 @@ _bfd_final_link_relocate (reloc_howto_ty
if (!bfd_reloc_offset_in_range (howto, input_bfd, input_section, octets))
return bfd_reloc_outofrange;
- /* This function assumes that we are dealing with a basic relocation
- against a symbol. We want to compute the value of the symbol to
- relocate to. This is just VALUE, the value of the symbol, plus
- ADDEND, any addend associated with the reloc. */
- relocation = value + addend;
-
/* If the relocation is PC relative, we want to set RELOCATION to
the distance between the symbol (currently in RELOCATION) and the
location we are relocating. Some targets (e.g., i386-aout)
@@ -1387,7 +1386,7 @@ _bfd_final_link_relocate (reloc_howto_ty
relocation -= address;
}
- return _bfd_relocate_contents (howto, input_bfd, relocation,
+ return _bfd_relocate_contents (howto, input_bfd, relocation, addend,
contents + octets);
}
@@ -1397,6 +1396,7 @@ bfd_reloc_status_type
_bfd_relocate_contents (reloc_howto_type *howto,
bfd *input_bfd,
bfd_vma relocation,
+ bfd_signed_vma addend,
bfd_byte *location)
{
bfd_vma x;
@@ -1404,12 +1404,18 @@ _bfd_relocate_contents (reloc_howto_type
unsigned int rightshift = howto->rightshift;
unsigned int bitpos = howto->bitpos;
+ if (howto->partial_inplace)
+ {
+ relocation += addend;
+ /* Get the value we are going to relocate. */
+ x = read_reloc (input_bfd, location, howto);
+ }
+ else
+ x = addend;
+
if (howto->negate)
relocation = -relocation;
- /* Get the value we are going to relocate. */
- x = read_reloc (input_bfd, location, howto);
-
/* Check for overflow. FIXME: We may drop bits during the addition
which we don't check for. We must either check at every single
operation, which would be tedious, or we must do the computations
--- a/bfd/xcofflink.c
+++ b/bfd/xcofflink.c
@@ -5777,7 +5777,7 @@ xcoff_reloc_link_order (bfd *output_bfd,
if (buf == NULL && size != 0)
return FALSE;
- rstat = _bfd_relocate_contents (howto, output_bfd, addend, buf);
+ rstat = _bfd_relocate_contents (howto, output_bfd, 0, addend, buf);
switch (rstat)
{
case bfd_reloc_ok:
More information about the Binutils
mailing list