Re: [Libunwind-devel] [PATCH] Check that the CIE is within the segment

[Peter Wu]

ping?

On Tuesday 25 November 2014 22:10:33 Peter Wu wrote:
> Due to a bug in the gold linker[1], the .eh_frame and .eh_frame_hdr
> sections contains garbage. When dwarf_extract_proc_info_from_fde tried
> to look up the begin of the CIE subsection, it would underflow the
> .eh_frame segment, resulting in a crash[2].
> 
> This patch avoids that crash by checking whether the CIE pointer is
> located after the begin of the .eh_frame section. The variable "base"
> was misused in various places as a boolean (decode as .debug_frame or
> decode as .eh_frame). These instances have been renamed to
> is_debug_frame where applicable.
> 
> Tested on Linux x86_64.
> 
>  [1]: https://sourceware.org/bugzilla/show_bug.cgi?id=17639
>  [2]: 
> http://lists.nongnu.org/archive/html/libunwind-devel/2014-11/msg00009.html
> 
> Signed-off-by: Peter Wu <address@hidden>
> ---
>  include/dwarf.h                 |  3 ++-
>  src/dwarf/Gfde.c                | 27 ++++++++++++++++++---------
>  src/dwarf/Gfind_proc_info-lsb.c | 12 ++++++++----
>  3 files changed, 28 insertions(+), 14 deletions(-)
> 
> diff --git a/include/dwarf.h b/include/dwarf.h
> index 169f739..633868b 100644
> --- a/include/dwarf.h
> +++ b/include/dwarf.h
> @@ -423,8 +423,9 @@ extern int dwarf_extract_proc_info_from_fde 
> (unw_addr_space_t as,
>                                               unw_accessors_t *a,
>                                               unw_word_t *fde_addr,
>                                               unw_proc_info_t *pi,
> -                                             int need_unwind_info,
>                                               unw_word_t base,
> +                                             int need_unwind_info,
> +                                             int is_debug_frame,
>                                               void *arg);
>  extern int dwarf_find_save_locs (struct dwarf_cursor *c);
>  extern int dwarf_create_state_record (struct dwarf_cursor *c,
> diff --git a/src/dwarf/Gfde.c b/src/dwarf/Gfde.c
> index dd00b62..5bcaa4d 100644
> --- a/src/dwarf/Gfde.c
> +++ b/src/dwarf/Gfde.c
> @@ -45,7 +45,7 @@ is_cie_id (unw_word_t val, int is_debug_frame)
>  static inline int
>  parse_cie (unw_addr_space_t as, unw_accessors_t *a, unw_word_t addr,
>             const unw_proc_info_t *pi, struct dwarf_cie_info *dci,
> -           unw_word_t base, void *arg)
> +           int is_debug_frame, void *arg)
>  {
>    uint8_t version, ch, augstr[5], fde_encoding, handler_encoding;
>    unw_word_t len, cie_end_addr, aug_size;
> @@ -79,7 +79,7 @@ parse_cie (unw_addr_space_t as, unw_accessors_t *a, 
> unw_word_t addr,
>        /* the CIE is in the 32-bit DWARF format */
>        uint32_t cie_id;
>        /* DWARF says CIE id should be 0xffffffff, but in .eh_frame, it's 0 */
> -      const uint32_t expected_id = (base) ? 0xffffffff : 0;
> +      const uint32_t expected_id = (is_debug_frame) ? 0xffffffff : 0;
>  
>        len = u32val;
>        cie_end_addr = addr + len;
> @@ -97,7 +97,7 @@ parse_cie (unw_addr_space_t as, unw_accessors_t *a, 
> unw_word_t addr,
>        uint64_t cie_id;
>        /* DWARF says CIE id should be 0xffffffffffffffff, but in
>           .eh_frame, it's 0 */
> -      const uint64_t expected_id = (base) ? 0xffffffffffffffffull : 0;
> +      const uint64_t expected_id = (is_debug_frame) ? 0xffffffffffffffffull 
> : 0;
>  
>        if ((ret = dwarf_readu64 (as, a, &addr, &u64val, arg)) < 0)
>          return ret;
> @@ -220,7 +220,8 @@ parse_cie (unw_addr_space_t as, unw_accessors_t *a, 
> unw_word_t addr,
>  HIDDEN int
>  dwarf_extract_proc_info_from_fde (unw_addr_space_t as, unw_accessors_t *a,
>                                    unw_word_t *addrp, unw_proc_info_t *pi,
> -                                  int need_unwind_info, unw_word_t base,
> +                                  unw_word_t base,
> +                                  int need_unwind_info, int is_debug_frame,
>                                    void *arg)
>  {
>    unw_word_t fde_end_addr, cie_addr, cie_offset_addr, aug_end_addr = 0;
> @@ -251,14 +252,18 @@ dwarf_extract_proc_info_from_fde (unw_addr_space_t as, 
> unw_accessors_t *a,
>        *addrp = fde_end_addr = addr + u32val;
>        cie_offset_addr = addr;
>  
> +      /* CIE must be within the segment. */
> +      if (cie_offset_addr < base)
> +          return -UNW_ENOINFO;
> +
>        if ((ret = dwarf_reads32 (as, a, &addr, &cie_offset, arg)) < 0)
>          return ret;
>  
> -      if (is_cie_id (cie_offset, base != 0))
> +      if (is_cie_id (cie_offset, is_debug_frame))
>          /* ignore CIEs (happens during linear searches) */
>          return 0;
>  
> -      if (base != 0)
> +      if (is_debug_frame)
>          cie_addr = base + cie_offset;
>        else
>          /* DWARF says that the CIE_pointer in the FDE is a
> @@ -279,14 +284,18 @@ dwarf_extract_proc_info_from_fde (unw_addr_space_t as, 
> unw_accessors_t *a,
>        *addrp = fde_end_addr = addr + u64val;
>        cie_offset_addr = addr;
>  
> +      /* CIE must be within the segment. */
> +      if (cie_offset_addr < base)
> +          return -UNW_ENOINFO;
> +
>        if ((ret = dwarf_reads64 (as, a, &addr, &cie_offset, arg)) < 0)
>          return ret;
>  
> -      if (is_cie_id (cie_offset, base != 0))
> +      if (is_cie_id (cie_offset, is_debug_frame))
>          /* ignore CIEs (happens during linear searches) */
>          return 0;
>  
> -      if (base != 0)
> +      if (is_debug_frame)
>          cie_addr = base + cie_offset;
>        else
>          /* DWARF says that the CIE_pointer in the FDE is a
> @@ -298,7 +307,7 @@ dwarf_extract_proc_info_from_fde (unw_addr_space_t as, 
> unw_accessors_t *a,
>  
>    Debug (15, "looking for CIE at address %lx\n", (long) cie_addr);
>  
> -  if ((ret = parse_cie (as, a, cie_addr, pi, &dci, base, arg)) < 0)
> +  if ((ret = parse_cie (as, a, cie_addr, pi, &dci, is_debug_frame, arg)) < 0)
>      return ret;
>  
>    /* IP-range has same encoding as FDE pointers, except that it's
> diff --git a/src/dwarf/Gfind_proc_info-lsb.c b/src/dwarf/Gfind_proc_info-lsb.c
> index e1bfbe7..32f165a 100644
> --- a/src/dwarf/Gfind_proc_info-lsb.c
> +++ b/src/dwarf/Gfind_proc_info-lsb.c
> @@ -59,8 +59,9 @@ linear_search (unw_addr_space_t as, unw_word_t ip,
>    while (i++ < fde_count && addr < eh_frame_end)
>      {
>        fde_addr = addr;
> -      if ((ret = dwarf_extract_proc_info_from_fde (as, a, &addr, pi, 0, 0, 
> arg))
> -          < 0)
> +      if ((ret = dwarf_extract_proc_info_from_fde (as, a, &addr, pi,
> +                                                   eh_frame_start,
> +                                                   0, 0, arg)) < 0)
>          return ret;
>  
>        if (ip >= pi->start_ip && ip < pi->end_ip)
> @@ -69,6 +70,7 @@ linear_search (unw_addr_space_t as, unw_word_t ip,
>              return 1;
>            addr = fde_addr;
>            if ((ret = dwarf_extract_proc_info_from_fde (as, a, &addr, pi,
> +                                                       eh_frame_start,
>                                                         need_unwind_info, 0,
>                                                         arg))
>                < 0)
> @@ -468,8 +470,8 @@ dwarf_find_debug_frame (int found, unw_dyn_info_t 
> *di_debug, unw_word_t ip,
>  
>                   err = dwarf_extract_proc_info_from_fde 
> (unw_local_addr_space,
>                                                           a, &fde_addr,
> -                                                         &this_pi, 0,
> -                                                         (uintptr_t) buf,
> +                                                         &this_pi,
> +                                                         (uintptr_t) buf, 0, 
> 1,
>                                                           NULL);
>                   if (err == 0)
>                     {
> @@ -902,6 +904,8 @@ dwarf_search_unwind_table (unw_addr_space_t as, 
> unw_word_t ip,
>              "fde_addr = %lx\n", (long) e->fde_offset, (long) segbase,
>              (long) debug_frame_base, (long) fde_addr);
>    if ((ret = dwarf_extract_proc_info_from_fde (as, a, &fde_addr, pi,
> +                                               debug_frame_base ?
> +                                               debug_frame_base : segbase,
>                                                 need_unwind_info,
>                                                 debug_frame_base, arg)) < 0)
>      return ret;
>