[PATCH] D43958: [llvm-readobj][ELF] Move ELF note parsing into lib/Object

[via llvm-commits]

Hi Rafael,

There is no bound to reference here; only in the context of the 
containing section or program header are there bounds to check.

Unfortunately that means they are only checked when using the 
corresponding _Range's and _Iterator's, but I do not know how best to 
avoid that, or if we need to.

In the usual case (paraphrasing):

   for (Header : Obj->program_headers())
     for (Note : Obj->notes(Header))
       outs() << Note->getName();

The program headers are checked to be in bounds of the binary (by 
program_headers(), as the headers are fixed length), and the notes are 
checked to be in bounds of their containing program header (by the 
iterator, as the notes are variable length).

However, looking back over the code I realize I only check the bounds of 
notes when incrementing the iterator, so the first one is not checked 
until after it is already used. I will update the patch to check all 
notes.

Regards,
Scott

On 2018-03-01 13:54, Rafael Avila de Espindola wrote:
> Scott Linder via Phabricator <reviews at reviews.llvm.org> writes:
> 
> 
>> +/// Note header
>> +template <class ELFT>
>> +struct Elf_Nhdr_Impl {
>> +  LLVM_ELF_IMPORT_TYPES_ELFT(ELFT)
>> +  Elf_Word n_namesz;
>> +  Elf_Word n_descsz;
>> +  Elf_Word n_type;
>> +
>> +  /// The alignment of the name and descriptor.
>> +  ///
>> +  /// Implementations differ from the specification here: in practice 
>> all
>> +  /// variants align both the name and descriptor to 4-bytes.
>> +  static const unsigned int Align = 4;
>> +
>> +  /// Get the note's name, excluding the terminating null byte.
>> +  const StringRef getName() const {
>> +    if (!n_namesz)
>> +      return StringRef();
>> +    return StringRef(reinterpret_cast<const char *>(this) +
>> +                         sizeof(Elf_Nhdr_Impl<ELFT>),
>> +                     n_namesz - 1);
> 
> Should we perform bounds checking in here?
> 
> Cheers,
> Rafael