c41d59
commit 801af9fafd4689337ebf27260aa115335a0cb2bc
c41d59
Author: Леонид Юрьев (Leonid Yuriev) <leo@yuriev.ru>
c41d59
Date:   Sat Feb 4 14:41:38 2023 +0300
c41d59
c41d59
    gmon: Fix allocated buffer overflow (bug 29444)
c41d59
    
c41d59
    The `__monstartup()` allocates a buffer used to store all the data
c41d59
    accumulated by the monitor.
c41d59
    
c41d59
    The size of this buffer depends on the size of the internal structures
c41d59
    used and the address range for which the monitor is activated, as well
c41d59
    as on the maximum density of call instructions and/or callable functions
c41d59
    that could be potentially on a segment of executable code.
c41d59
    
c41d59
    In particular a hash table of arcs is placed at the end of this buffer.
c41d59
    The size of this hash table is calculated in bytes as
c41d59
       p->fromssize = p->textsize / HASHFRACTION;
c41d59
    
c41d59
    but actually should be
c41d59
       p->fromssize = ROUNDUP(p->textsize / HASHFRACTION, sizeof(*p->froms));
c41d59
    
c41d59
    This results in writing beyond the end of the allocated buffer when an
c41d59
    added arc corresponds to a call near from the end of the monitored
c41d59
    address range, since `_mcount()` check the incoming caller address for
c41d59
    monitored range but not the intermediate result hash-like index that
c41d59
    uses to write into the table.
c41d59
    
c41d59
    It should be noted that when the results are output to `gmon.out`, the
c41d59
    table is read to the last element calculated from the allocated size in
c41d59
    bytes, so the arcs stored outside the buffer boundary did not fall into
c41d59
    `gprof` for analysis. Thus this "feature" help me to found this bug
c41d59
    during working with https://sourceware.org/bugzilla/show_bug.cgi?id=29438
c41d59
    
c41d59
    Just in case, I will explicitly note that the problem breaks the
c41d59
    `make test t=gmon/tst-gmon-dso` added for Bug 29438.
c41d59
    There, the arc of the `f3()` call disappears from the output, since in
c41d59
    the DSO case, the call to `f3` is located close to the end of the
c41d59
    monitored range.
c41d59
    
c41d59
    Signed-off-by: Леонид Юрьев (Leonid Yuriev) <leo@yuriev.ru>
c41d59
    
c41d59
    Another minor error seems a related typo in the calculation of
c41d59
    `kcountsize`, but since kcounts are smaller than froms, this is
c41d59
    actually to align the p->froms data.
c41d59
    
c41d59
    Co-authored-by: DJ Delorie <dj@redhat.com>
c41d59
    Reviewed-by: Carlos O'Donell <carlos@redhat.com>
c41d59
c41d59
diff --git a/gmon/gmon.c b/gmon/gmon.c
c41d59
index dee64803ada583d7..bf76358d5b1aa2da 100644
c41d59
--- a/gmon/gmon.c
c41d59
+++ b/gmon/gmon.c
c41d59
@@ -132,6 +132,8 @@ __monstartup (u_long lowpc, u_long highpc)
c41d59
   p->lowpc = ROUNDDOWN(lowpc, HISTFRACTION * sizeof(HISTCOUNTER));
c41d59
   p->highpc = ROUNDUP(highpc, HISTFRACTION * sizeof(HISTCOUNTER));
c41d59
   p->textsize = p->highpc - p->lowpc;
c41d59
+  /* This looks like a typo, but it's here to align the p->froms
c41d59
+     section.  */
c41d59
   p->kcountsize = ROUNDUP(p->textsize / HISTFRACTION, sizeof(*p->froms));
c41d59
   p->hashfraction = HASHFRACTION;
c41d59
   p->log_hashfraction = -1;
c41d59
@@ -142,7 +144,7 @@ __monstartup (u_long lowpc, u_long highpc)
c41d59
 	 instead of integer division.  Precompute shift amount. */
c41d59
       p->log_hashfraction = ffs(p->hashfraction * sizeof(*p->froms)) - 1;
c41d59
   }
c41d59
-  p->fromssize = p->textsize / HASHFRACTION;
c41d59
+  p->fromssize = ROUNDUP(p->textsize / HASHFRACTION, sizeof(*p->froms));
c41d59
   p->tolimit = p->textsize * ARCDENSITY / 100;
c41d59
   if (p->tolimit < MINARCS)
c41d59
     p->tolimit = MINARCS;