public inbox for glibc-cvs@sourceware.org
help / color / mirror / Atom feed
From: DJ Delorie <dj@sourceware.org>
To: glibc-cvs@sourceware.org
Subject: [glibc] gmon: Fix allocated buffer overflow (bug 29444)
Date: Wed, 22 Feb 2023 22:24:25 +0000 (GMT)	[thread overview]
Message-ID: <20230222222425.89F9C3858D33@sourceware.org> (raw)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=801af9fafd4689337ebf27260aa115335a0cb2bc

commit 801af9fafd4689337ebf27260aa115335a0cb2bc
Author: Леонид Юрьев (Leonid Yuriev) <leo@yuriev.ru>
Date:   Sat Feb 4 14:41:38 2023 +0300

    gmon: Fix allocated buffer overflow (bug 29444)
    
    The `__monstartup()` allocates a buffer used to store all the data
    accumulated by the monitor.
    
    The size of this buffer depends on the size of the internal structures
    used and the address range for which the monitor is activated, as well
    as on the maximum density of call instructions and/or callable functions
    that could be potentially on a segment of executable code.
    
    In particular a hash table of arcs is placed at the end of this buffer.
    The size of this hash table is calculated in bytes as
       p->fromssize = p->textsize / HASHFRACTION;
    
    but actually should be
       p->fromssize = ROUNDUP(p->textsize / HASHFRACTION, sizeof(*p->froms));
    
    This results in writing beyond the end of the allocated buffer when an
    added arc corresponds to a call near from the end of the monitored
    address range, since `_mcount()` check the incoming caller address for
    monitored range but not the intermediate result hash-like index that
    uses to write into the table.
    
    It should be noted that when the results are output to `gmon.out`, the
    table is read to the last element calculated from the allocated size in
    bytes, so the arcs stored outside the buffer boundary did not fall into
    `gprof` for analysis. Thus this "feature" help me to found this bug
    during working with https://sourceware.org/bugzilla/show_bug.cgi?id=29438
    
    Just in case, I will explicitly note that the problem breaks the
    `make test t=gmon/tst-gmon-dso` added for Bug 29438.
    There, the arc of the `f3()` call disappears from the output, since in
    the DSO case, the call to `f3` is located close to the end of the
    monitored range.
    
    Signed-off-by: Леонид Юрьев (Leonid Yuriev) <leo@yuriev.ru>
    
    Another minor error seems a related typo in the calculation of
    `kcountsize`, but since kcounts are smaller than froms, this is
    actually to align the p->froms data.
    
    Co-authored-by: DJ Delorie <dj@redhat.com>
    Reviewed-by: Carlos O'Donell <carlos@redhat.com>

Diff:
---
 gmon/gmon.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/gmon/gmon.c b/gmon/gmon.c
index dee64803ad..bf76358d5b 100644
--- a/gmon/gmon.c
+++ b/gmon/gmon.c
@@ -132,6 +132,8 @@ __monstartup (u_long lowpc, u_long highpc)
   p->lowpc = ROUNDDOWN(lowpc, HISTFRACTION * sizeof(HISTCOUNTER));
   p->highpc = ROUNDUP(highpc, HISTFRACTION * sizeof(HISTCOUNTER));
   p->textsize = p->highpc - p->lowpc;
+  /* This looks like a typo, but it's here to align the p->froms
+     section.  */
   p->kcountsize = ROUNDUP(p->textsize / HISTFRACTION, sizeof(*p->froms));
   p->hashfraction = HASHFRACTION;
   p->log_hashfraction = -1;
@@ -142,7 +144,7 @@ __monstartup (u_long lowpc, u_long highpc)
 	 instead of integer division.  Precompute shift amount. */
       p->log_hashfraction = ffs(p->hashfraction * sizeof(*p->froms)) - 1;
   }
-  p->fromssize = p->textsize / HASHFRACTION;
+  p->fromssize = ROUNDUP(p->textsize / HASHFRACTION, sizeof(*p->froms));
   p->tolimit = p->textsize * ARCDENSITY / 100;
   if (p->tolimit < MINARCS)
     p->tolimit = MINARCS;

                 reply	other threads:[~2023-02-22 22:24 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230222222425.89F9C3858D33@sourceware.org \
    --to=dj@sourceware.org \
    --cc=glibc-cvs@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).