[libunwind] [libunwind] Avoid reading OOB for non-existent .eh_frame_hdr (PR #68815)

[Alexander Richardson via cfe-commits]

https://github.com/arichardson updated https://github.com/llvm/llvm-project/pull/68815

>From ff360ee7f304424dd0d12d00b8c0ba6801241410 Mon Sep 17 00:00:00 2001
From: Alex Richardson <alexrichardson at google.com>
Date: Wed, 11 Oct 2023 08:34:55 -0700
Subject: [PATCH 1/2] [libunwind] Fix wrong end argument passed to
 decodeEHHdr()

All but one callsite were actually passing start+length arguments.
This should not have any functional change since the end argument is
almost always ignored.
I noticed this while debugging some incorrect error messages being
printed while running the testsuite baremetal (using binaries that did
not have a valid eh_frame_hdr section): the tests print
`libunwind: unsupported .eh_frame_hdr version: 20 at 8000d308` because
libunwind is reading nonsense data for .eh_frame_hdr.
---
 libunwind/src/AddressSpace.hpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libunwind/src/AddressSpace.hpp b/libunwind/src/AddressSpace.hpp
index 1abbc822546878d..5551c7d4bef1c56 100644
--- a/libunwind/src/AddressSpace.hpp
+++ b/libunwind/src/AddressSpace.hpp
@@ -414,8 +414,8 @@ static bool checkForUnwindInfoSegment(const Elf_Phdr *phdr, size_t image_base,
     cbdata->sects->dwarf_index_section = eh_frame_hdr_start;
     cbdata->sects->dwarf_index_section_length = phdr->p_memsz;
     if (EHHeaderParser<LocalAddressSpace>::decodeEHHdr(
-            *cbdata->addressSpace, eh_frame_hdr_start, phdr->p_memsz,
-            hdrInfo)) {
+            *cbdata->addressSpace, eh_frame_hdr_start,
+            eh_frame_hdr_start + phdr->p_memsz, hdrInfo)) {
       // .eh_frame_hdr records the start of .eh_frame, but not its size.
       // Rely on a zero terminator to find the end of the section.
       cbdata->sects->dwarf_section = hdrInfo.eh_frame_ptr;
@@ -638,7 +638,8 @@ inline bool LocalAddressSpace::findUnwindSections(pint_t targetAddr,
     info.dwarf_index_section_length = SIZE_MAX;
     EHHeaderParser<LocalAddressSpace>::EHHeaderInfo hdrInfo;
     if (!EHHeaderParser<LocalAddressSpace>::decodeEHHdr(
-            *this, info.dwarf_index_section, info.dwarf_index_section_length,
+            *this, info.dwarf_index_section,
+            info.dwarf_index_section + info.dwarf_index_section_length,
             hdrInfo)) {
       return false;
     }

>From 0190d73d214feb1737206223ae44d9fced51de4d Mon Sep 17 00:00:00 2001
From: Alex Richardson <alexrichardson at google.com>
Date: Wed, 11 Oct 2023 08:52:45 -0700
Subject: [PATCH 2/2] [libunwind] Avoid reading OOB for non-existent
 .eh_frame_hdr

I was running the tests with baremetal picolibc which has a linker
script that __eh_frame_start==__eh_frame_end (not equal to zero) in
case there is no .eh_frame_hdr.
I noticed that libunwind was trying to read nonsense data because it
was printing messages such as
`libunwind: unsupported .eh_frame_hdr version: 20 at 8000d308`

This change adds a ehHdrSize check to avoid reading this out-of-bounds
data and potentially crashing.
---
 libunwind/src/EHHeaderParser.hpp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/libunwind/src/EHHeaderParser.hpp b/libunwind/src/EHHeaderParser.hpp
index ed4317c89055c9e..edc77daae627e67 100644
--- a/libunwind/src/EHHeaderParser.hpp
+++ b/libunwind/src/EHHeaderParser.hpp
@@ -55,6 +55,18 @@ template <typename A>
 bool EHHeaderParser<A>::decodeEHHdr(A &addressSpace, pint_t ehHdrStart,
                                     pint_t ehHdrEnd, EHHeaderInfo &ehHdrInfo) {
   pint_t p = ehHdrStart;
+
+  // Ensure that we don't read data beyond the end of .eh_frame_hdr
+  if (ehHdrEnd - ehHdrStart < 4) {
+    // Don't print a message for an empty .eh_frame_hdr (this can happen if
+    // the linker script defines symbols for it even in the empty case).
+    if (ehHdrEnd == ehHdrStart)
+      return false;
+    _LIBUNWIND_LOG("unsupported .eh_frame_hdr at %" PRIx64
+                   ": need at least 4 bytes of data but only got %zd",
+                   static_cast<uint64_t>(ehHdrStart), ehHdrSize);
+    return false;
+  }
   uint8_t version = addressSpace.get8(p++);
   if (version != 1) {
     _LIBUNWIND_LOG("unsupported .eh_frame_hdr version: %" PRIu8 " at %" PRIx64,