* [PATCH 1/2 v2] Don't overflow in __libdw_in_section @ 2017-12-08 15:05 Ulf Hermann 2017-12-14 13:43 ` Mark Wielaard 0 siblings, 1 reply; 6+ messages in thread From: Ulf Hermann @ 2017-12-08 15:05 UTC (permalink / raw) To: elfutils-devel This exposes a bug in dwarf_formstring as detected by the dwarf-getmacros test. We cannot unconditionally assume that a string is in either the IDX_debug_info or the IDX_debug_types section as determined by cu_sec_idx. (Signed-off instead of Change-Id ...) Signed-off-by: Ulf Hermann <ulf.hermann@qt.io> --- libdw/ChangeLog | 4 ++++ libdw/libdwP.h | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/libdw/ChangeLog b/libdw/ChangeLog index 4375244..996cd2e 100644 --- a/libdw/ChangeLog +++ b/libdw/ChangeLog @@ -1,3 +1,7 @@ +2017-05-09 Ulf Hermann <ulf.hermann@qt.io> + + * libdwP.h: Fix check for the upper border of the range in __libdw_in_section. + 2017-11-03 Mark Wielaard <mark@klomp.org> * dwarf_getlocation.c (__libdw_intern_expression): Handle diff --git a/libdw/libdwP.h b/libdw/libdwP.h index 78c0013..e092d8e 100644 --- a/libdw/libdwP.h +++ b/libdw/libdwP.h @@ -643,7 +643,8 @@ __libdw_in_section (Dwarf *dbg, int sec_index, if (data == NULL) return false; if (unlikely (addr < data->d_buf) - || unlikely (data->d_size - (addr - data->d_buf) < size)) + || unlikely (data->d_size < size) + || unlikely ((size_t)(addr - data->d_buf) > data->d_size - size)) { __libdw_seterrno (DWARF_E_INVALID_OFFSET); return false; -- 2.8.1.windows.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2 v2] Don't overflow in __libdw_in_section 2017-12-08 15:05 [PATCH 1/2 v2] Don't overflow in __libdw_in_section Ulf Hermann @ 2017-12-14 13:43 ` Mark Wielaard 2017-12-14 13:55 ` Ulf Hermann 0 siblings, 1 reply; 6+ messages in thread From: Mark Wielaard @ 2017-12-14 13:43 UTC (permalink / raw) To: Ulf Hermann, elfutils-devel Hi Ulf, (Meta, I have some trouble applying this with git am, it thinks the patch is malformed. But I can apply by hand of course.) On Fri, 2017-12-08 at 16:05 +0100, Ulf Hermann wrote: > > +2017-05-09 Ulf Hermann <ulf.hermann@qt.io> > + > + * libdwP.h: Fix check for the upper border of the range in > __libdw_in_section. > + > 2017-11-03 Mark Wielaard <mark@klomp.org> > * dwarf_getlocation.c (__libdw_intern_expression): Handle > diff --git a/libdw/libdwP.h b/libdw/libdwP.h > index 78c0013..e092d8e 100644 > --- a/libdw/libdwP.h > +++ b/libdw/libdwP.h > @@ -643,7 +643,8 @@ __libdw_in_section (Dwarf *dbg, int sec_index, > if (data == NULL) > return false; > if (unlikely (addr < data->d_buf) > - || unlikely (data->d_size - (addr - data->d_buf) < size)) > + || unlikely (data->d_size < size) > + || unlikely ((size_t)(addr - data->d_buf) > data->d_size - > size)) > { > __libdw_seterrno (DWARF_E_INVALID_OFFSET); > return false; The transformation seems correct. But if we can overflow/underflow here, do we have the same problem in __libdw_offset_in_section where we check data->d_size - offset < size, with offset a Dwarf_Off? Thanks, Mark ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2 v2] Don't overflow in __libdw_in_section 2017-12-14 13:43 ` Mark Wielaard @ 2017-12-14 13:55 ` Ulf Hermann 2017-12-20 18:05 ` Mark Wielaard 0 siblings, 1 reply; 6+ messages in thread From: Ulf Hermann @ 2017-12-14 13:55 UTC (permalink / raw) To: Mark Wielaard, elfutils-devel On 12/14/2017 02:43 PM, Mark Wielaard wrote: > (Meta, I have some trouble applying this with git am, it thinks the > patch is malformed. But I can apply by hand of course.) Oh, sorry for that. It's probably the leading spaces again. I keep messing up my mail setup on windows ... > The transformation seems correct. But if we can overflow/underflow > here, do we have the same problem in __libdw_offset_in_section where we > check data->d_size - offset < size, with offset a Dwarf_Off? Probably we have the same problem there. I didn't catch any instances of it, though. regards, Ulf ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2 v2] Don't overflow in __libdw_in_section 2017-12-14 13:55 ` Ulf Hermann @ 2017-12-20 18:05 ` Mark Wielaard 2017-12-21 13:47 ` Ulf Hermann 0 siblings, 1 reply; 6+ messages in thread From: Mark Wielaard @ 2017-12-20 18:05 UTC (permalink / raw) To: Ulf Hermann, elfutils-devel [-- Attachment #1: Type: text/plain, Size: 614 bytes --] On Thu, 2017-12-14 at 14:55 +0100, Ulf Hermann wrote: > On 12/14/2017 02:43 PM, Mark Wielaard wrote: > > The transformation seems correct. But if we can overflow/underflow > > here, do we have the same problem in __libdw_offset_in_section > > where we > > check data->d_size - offset < size, with offset a Dwarf_Off? > > Probably we have the same problem there. I didn't catch any instances > of it, though. It is surprising we didn't see more issues with this code. There is also the fake loc cu that fetches data from a different section. I updated both functions as attached. Cheers, Mark [-- Attachment #2: 0002-Don-t-overflow-in-__libdw_in_section-and-__libdw_off.patch --] [-- Type: text/x-patch, Size: 1929 bytes --] From 0d100f63db640c533748a7adaa099499b2d2d4b0 Mon Sep 17 00:00:00 2001 From: Ulf Hermann <ulf.hermann@qt.io> Date: Tue, 9 May 2017 18:28:33 +0200 Subject: [PATCH 2/2] Don't overflow in __libdw_in_section and __libdw_offset_in_section. This exposes a bug in dwarf_formstring as detected by the dwarf-getmacros test before we made sure to use the correct sec_idx for the CU. Signed-off-by: Ulf Hermann <ulf.hermann@qt.io> Signed-off-by: Mark Wielaard <mark@klomp.org> --- libdw/ChangeLog | 7 +++++++ libdw/libdwP.h | 6 ++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/libdw/ChangeLog b/libdw/ChangeLog index 22b7bf4..eb1cb70 100644 --- a/libdw/ChangeLog +++ b/libdw/ChangeLog @@ -1,3 +1,10 @@ +2017-05-09 Ulf Hermann <ulf.hermann@qt.io> + Mark Wielaard <mark@klomp.org> + + * libdwP.h (__libdw_in_section): Fix check for the upper border of + the range. + (__libdw_offset_in_section): Likewise. + 2017-12-20 Mark Wielaard <mark@klomp.org> * libdwP.h (struct Dwarf_CU): Add sec_idx field. diff --git a/libdw/libdwP.h b/libdw/libdwP.h index f524347..82b47d0 100644 --- a/libdw/libdwP.h +++ b/libdw/libdwP.h @@ -628,7 +628,8 @@ __libdw_offset_in_section (Dwarf *dbg, int sec_index, if (data == NULL) return -1; if (unlikely (offset > data->d_size) - || unlikely (data->d_size - offset < size)) + || unlikely (data->d_size < size) + || unlikely (offset > data->d_size - size)) { __libdw_seterrno (DWARF_E_INVALID_OFFSET); return -1; @@ -645,7 +646,8 @@ __libdw_in_section (Dwarf *dbg, int sec_index, if (data == NULL) return false; if (unlikely (addr < data->d_buf) - || unlikely (data->d_size - (addr - data->d_buf) < size)) + || unlikely (data->d_size < size) + || unlikely ((size_t)(addr - data->d_buf) > data->d_size - size)) { __libdw_seterrno (DWARF_E_INVALID_OFFSET); return false; -- 1.8.3.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2 v2] Don't overflow in __libdw_in_section 2017-12-20 18:05 ` Mark Wielaard @ 2017-12-21 13:47 ` Ulf Hermann 2017-12-21 20:22 ` Mark Wielaard 0 siblings, 1 reply; 6+ messages in thread From: Ulf Hermann @ 2017-12-21 13:47 UTC (permalink / raw) To: Mark Wielaard, elfutils-devel > It is surprising we didn't see more issues with this code. There is > also the fake loc cu that fetches data from a different section. I > updated both functions as attached. Looks good to me. Ulf ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/2 v2] Don't overflow in __libdw_in_section 2017-12-21 13:47 ` Ulf Hermann @ 2017-12-21 20:22 ` Mark Wielaard 0 siblings, 0 replies; 6+ messages in thread From: Mark Wielaard @ 2017-12-21 20:22 UTC (permalink / raw) To: Ulf Hermann; +Cc: elfutils-devel On Thu, Dec 21, 2017 at 02:47:49PM +0100, Ulf Hermann wrote: > > It is surprising we didn't see more issues with this code. There is > > also the fake loc cu that fetches data from a different section. I > > updated both functions as attached. > > Looks good to me. Thanks for taking a look. Pushed to master. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-12-21 20:22 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2017-12-08 15:05 [PATCH 1/2 v2] Don't overflow in __libdw_in_section Ulf Hermann 2017-12-14 13:43 ` Mark Wielaard 2017-12-14 13:55 ` Ulf Hermann 2017-12-20 18:05 ` Mark Wielaard 2017-12-21 13:47 ` Ulf Hermann 2017-12-21 20:22 ` Mark Wielaard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).