[PATCH] D86422: [ELF] .note.gnu.property: error for invalid pr_datasize

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: lld/ELF/InputFiles.cpp:795
     // Read one NOTE record.
-    if (data.size() < sizeof(Elf_Nhdr))
-      fatal(toString(obj) + ": .note.gnu.property: section too short");
-
     auto *nhdr = reinterpret_cast<const Elf_Nhdr *>(data.data());
+    if (data.size() < sizeof(Elf_Nhdr) || data.size() < nhdr->getSize())
----------------
psmith wrote:
> Will this work if the Host is little endian and the data in big-endian format?
I believe yes. `ELF_Nhdr` is `Elf_Nhdr_Impl`:


```
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;
...
```

And `Elf_Word `s here are `using Word = packed<uint32_t>` which respects
endianess:

```
struct packed_endian_specific_integral {
...
operator value_type() const {
  return endian::read<value_type, endian, alignment>(Ptr);
}
```

```
template <typename value_type, std::size_t alignment>
inline value_type read(const void *memory, endianness endian) {
  value_type ret;

  memcpy(&ret,
         LLVM_ASSUME_ALIGNED(
             memory, (detail::PickAlignment<value_type, alignment>::value)),
         sizeof(value_type));
  return byte_swap<value_type>(ret, endian);
}
```


```
template <typename value_type>
inline value_type byte_swap(value_type value, endianness endian) {
  if ((endian != native) && (endian != system_endianness()))
    sys::swapByteOrder(value);
  return value;
}
```


================
Comment at: lld/ELF/InputFiles.cpp:818
+      if (desc.size() < 8 + size)
+        reportFatal(desc, "program property is too short");
 
----------------
2 points:

1) You should be able to use `read32` from `Target.h`:

```
inline uint32_t read32(const void *p) {
  return llvm::support::endian::read32(p, config->endianness);
}
```

2) It probably would read slightly better if it was:

```
if (desc.size() < 8)
  reportFatal(...);

uint32_t type = read32(desc.data());
uint32_t size = read32(desc.data() + 4);
desc = desc.drop_front(8);

if (desc.size() < size)
  reportFatal(desc, "program property is too short");
```

i.e
a) I'd not start reading the `size` + `type` when the data is truncated (just like the code on the left).
b) Early dropping of 8 bytes at start should allow to avoid the need of `+ 8` in the code below.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86422/new/

https://reviews.llvm.org/D86422