|
|
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;
|