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

[Alexander Richardson via cfe-commits]

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

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 https://github.com/llvm/llvm-project/commit/8000d308146ebf49cb364cb600e28a0a42e22c83`

This change adds a ehHdrSize check to avoid reading this out-of-bounds
data and potentially crashing.

Depends on #68813 which I've included as the first commit here (not sure how to sensibly do stacked PRs).

>From f5b7f1fd65135412f24dc4dd7887911fb0e4c2ed 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] Consistently pass start+length to
 decodeEHHdr()

It previously took a start+end pint_t, but 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/EHHeaderParser.hpp | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/libunwind/src/EHHeaderParser.hpp b/libunwind/src/EHHeaderParser.hpp
index ed4317c89055c9e..2162178e10bb2fb 100644
--- a/libunwind/src/EHHeaderParser.hpp
+++ b/libunwind/src/EHHeaderParser.hpp
@@ -35,7 +35,7 @@ template <typename A> class EHHeaderParser {
     uint8_t table_enc;
   };
 
-  static bool decodeEHHdr(A &addressSpace, pint_t ehHdrStart, pint_t ehHdrEnd,
+  static bool decodeEHHdr(A &addressSpace, pint_t ehHdrStart, size_t ehHdrSize,
                           EHHeaderInfo &ehHdrInfo);
   static bool findFDE(A &addressSpace, pint_t pc, pint_t ehHdrStart,
                       uint32_t sectionLength,
@@ -53,8 +53,9 @@ template <typename A> class EHHeaderParser {
 
 template <typename A>
 bool EHHeaderParser<A>::decodeEHHdr(A &addressSpace, pint_t ehHdrStart,
-                                    pint_t ehHdrEnd, EHHeaderInfo &ehHdrInfo) {
+                                    size_t ehHdrSize, EHHeaderInfo &ehHdrInfo) {
   pint_t p = ehHdrStart;
+  pint_t ehHdrEnd = ehHdrStart + ehHdrSize;
   uint8_t version = addressSpace.get8(p++);
   if (version != 1) {
     _LIBUNWIND_LOG("unsupported .eh_frame_hdr version: %" PRIu8 " at %" PRIx64,
@@ -106,7 +107,7 @@ bool EHHeaderParser<A>::findFDE(A &addressSpace, pint_t pc, pint_t ehHdrStart,
   pint_t ehHdrEnd = ehHdrStart + sectionLength;
 
   EHHeaderParser<A>::EHHeaderInfo hdrInfo;
-  if (!EHHeaderParser<A>::decodeEHHdr(addressSpace, ehHdrStart, ehHdrEnd,
+  if (!EHHeaderParser<A>::decodeEHHdr(addressSpace, ehHdrStart, sectionLength,
                                       hdrInfo))
     return false;
 

>From cda672a5cd3369c07a8634831d28b6d07d3e8cad 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 2162178e10bb2fb..8d37594e7b9be89 100644
--- a/libunwind/src/EHHeaderParser.hpp
+++ b/libunwind/src/EHHeaderParser.hpp
@@ -56,6 +56,18 @@ bool EHHeaderParser<A>::decodeEHHdr(A &addressSpace, pint_t ehHdrStart,
                                     size_t ehHdrSize, EHHeaderInfo &ehHdrInfo) {
   pint_t p = ehHdrStart;
   pint_t ehHdrEnd = ehHdrStart + ehHdrSize;
+
+  // Ensure that we don't read data beyond the end of .eh_frame_hdr
+  if (ehHdrSize < 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 (ehHdrSize == 0)
+      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,