mbox series

[v2,0/1] RISC-V: Strict relocation handling

Message ID cover.1697675352.git.research_trasio@irq.a4lg.com
Headers
Series RISC-V: Strict relocation handling |

Message

Tsukasa OI Oct. 19, 2023, 12:30 a.m. UTC 
  Hi,

This is the PATCH v2 of an attempt to prevent future accidents possibly
caused by future relocation additions.

PATCH v1:
<https://sourceware.org/pipermail/binutils/2023-October/129971.html>


[Added in v2]

1.  Internal relocations are no longer emitted from the linker
    even if you use ld --emit-relocs option.


[Changes in v2]

1.  It became a single large patch since it's harder to separate
    semantically (and cleanly).
2.  Rejecting unknown relocs in riscv_elf_check_relocs is preserved but made
    simpler (no longer need to list all supported public relocations).
    I concluded that it was necessary (rare but valid as in FR-V) but there
    were a simpler way to perform the same operation.
3.  Wraps querying whether the relocation is empty, clarifying that we
    want to reject empty relocations (minor).
4.  Moves all internal relocations after R_RISCV_max.
    I'll describe this in detail.


[Move internal relocations after R_RISCV_max]

This method is suggested by Nelson and I tried.  I soon noticed that simply
moving internal relocations after R_RISCV_max doesn't work.

It was due to the fact that handling of R_RISCV_DELETE (the only internal
relocation defined relative to R_RISCV_max) was so special so that moving
other relocations causes problems.

Specifically, riscv_elf_relocate_section function (the function finally
relocates the section) calls riscv_elf_rtype_to_howto and requires generic
howto information.  Yes, howto information.

So my second attempt is: let riscv_elf_rtype_to_howto search through the
special howto table only containing internal relocations only when required.

As far as I know, the only location we need the internal relocations is
in the riscv_elf_relocate_section function and new lookup_internal argument
is set to true only here (I think adding this argument is the simplest
because this function calls the error handler if fails and creating separate
internal variant will require duplicating most of the code).

... and that was a success!

Internal relocations are now all relative to R_RISCV_max and if a relocation
type is added after the biggest known relocation, internal relocation type
numbers are automatically increased.  I found an interesting example in
libctf/swap.h so I added bonus safety measure to make sure that such
internal relocations are always safe to handle (relocation type numbers must
fit in an 8-bit integer due to constraints in ELF32).


I hope I resolved most of the issues Nelson pointed out.


Sincerely,
Tsukasa




Tsukasa OI (1):
  RISC-V: Separate invalid/internal only ELF relocs

 bfd/elfnn-riscv.c   |  40 +++++----
 bfd/elfxx-riscv.c   | 202 +++++++++++++++++++++++++++-----------------
 bfd/elfxx-riscv.h   |   3 +-
 include/elf/riscv.h |  37 ++++++--
 4 files changed, 183 insertions(+), 99 deletions(-)


base-commit: e734b3e980d8a30ea23b5640d871b59d33720ecf