gmon: fix memory corruption issues [BZ# 30101]

V2 of this patch fixes an issue in V1, where the state was changed to ON not
OFF at end of _mcleanup. I hadn't noticed that (counterintuitively) ON=0 and
OFF=3, hence zeroing the buffer turned it back on. So set the state to OFF
after the memset.

1. Prevent double free, and reads from unallocated memory, when
   _mcleanup is (incorrectly) called two or more times in a row,
   without an intervening call to __monstartup; with this patch, the
   second and subsequent calls effectively become no-ops instead.
   While setting tos=NULL is minimal fix, safest action is to zero the
   whole gmonparam buffer.

2. Prevent memory leak when __monstartup is (incorrectly) called two
   or more times in a row, without an intervening call to _mcleanup;
   with this patch, the second and subsequent calls effectively become
   no-ops instead.

3. After _mcleanup, treat __moncontrol(1) as __moncontrol(0) instead.
   With zeroing of gmonparam buffer in _mcleanup, this stops the
   state incorrectly being changed to GMON_PROF_ON despite profiling
   actually being off. If we'd just done the minimal fix to _mcleanup
   of setting tos=NULL, there is risk of far worse memory corruption:
   kcount would point to deallocated memory, and the __profil syscall
   would make the kernel write profiling data into that memory,
   which could have since been reallocated to something unrelated.

4. Ensure __moncontrol(0) still turns off profiling even in error
   state. Otherwise, if mcount overflows and sets state to
   GMON_PROF_ERROR, when _mcleanup calls __moncontrol(0), the __profil
   syscall to disable profiling will not be invoked. _mcleanup will
   free the buffer, but the kernel will still be writing profiling
   data into it, potentially corrupted arbitrary memory.

Also adds a test case for (1). Issues (2)-(4) are not feasible to test.

Signed-off-by: Simon Kissane <skissane@gmail.com>
Reviewed-by: DJ Delorie <dj@redhat.com>
This commit is contained in:
Simon Kissane 2023-02-11 08:58:02 +11:00 committed by DJ Delorie
parent 31be941e43
commit bde1218720
3 changed files with 64 additions and 8 deletions

View File

@ -1,4 +1,5 @@
# Copyright (C) 1995-2023 Free Software Foundation, Inc.
# Copyright The GNU Toolchain Authors.
# This file is part of the GNU C Library.
# The GNU C Library is free software; you can redistribute it and/or
@ -25,7 +26,7 @@ include ../Makeconfig
headers := sys/gmon.h sys/gmon_out.h sys/profil.h
routines := gmon mcount profil sprofil prof-freq
tests = tst-sprofil tst-gmon tst-mcount-overflow
tests = tst-sprofil tst-gmon tst-mcount-overflow tst-mcleanup
ifeq ($(build-profile),yes)
tests += tst-profile-static
tests-static += tst-profile-static
@ -68,6 +69,14 @@ ifeq ($(run-built-tests),yes)
tests-special += $(objpfx)tst-mcount-overflow-check.out
endif
CFLAGS-tst-mcleanup.c := -fno-omit-frame-pointer -pg
tst-mcleanup-no-pie = yes
CRT-tst-mcleanup := $(csu-objpfx)g$(start-installed-name)
tst-mcleanup-ENV := GMON_OUT_PREFIX=$(objpfx)tst-mcleanup.data
ifeq ($(run-built-tests),yes)
tests-special += $(objpfx)tst-mcleanup.out
endif
CFLAGS-tst-gmon-static.c := $(PIE-ccflag) -fno-omit-frame-pointer -pg
CRT-tst-gmon-static := $(csu-objpfx)g$(static-start-installed-name)
tst-gmon-static-no-pie = yes
@ -123,6 +132,10 @@ $(objpfx)tst-mcount-overflow-check.out: tst-mcount-overflow-check.sh $(objpfx)ts
$(SHELL) $< $(objpfx)tst-mcount-overflow > $@; \
$(evaluate-test)
$(objpfx)tst-mcleanup.out: clean-tst-mcleanup-data
clean-tst-mcleanup-data:
rm -f $(objpfx)tst-mcleanup.data.*
$(objpfx)tst-gmon-gprof.out: tst-gmon-gprof.sh $(objpfx)tst-gmon.out
$(SHELL) $< $(GPROF) $(objpfx)tst-gmon $(objpfx)tst-gmon.data.* > $@; \
$(evaluate-test)

View File

@ -102,11 +102,8 @@ __moncontrol (int mode)
{
struct gmonparam *p = &_gmonparam;
/* Don't change the state if we ran into an error. */
if (p->state == GMON_PROF_ERROR)
return;
if (mode)
/* Treat start request as stop if error or gmon not initialized. */
if (mode && p->state != GMON_PROF_ERROR && p->tos != NULL)
{
/* start */
__profil((void *) p->kcount, p->kcountsize, p->lowpc, s_scale);
@ -116,6 +113,8 @@ __moncontrol (int mode)
{
/* stop */
__profil(NULL, 0, 0, 0);
/* Don't change the state if we ran into an error. */
if (p->state != GMON_PROF_ERROR)
p->state = GMON_PROF_OFF;
}
}
@ -146,6 +145,14 @@ __monstartup (u_long lowpc, u_long highpc)
maxarcs = MAXARCS;
#endif
/*
* If we are incorrectly called twice in a row (without an
* intervening call to _mcleanup), ignore the second call to
* prevent leaking memory.
*/
if (p->tos != NULL)
return;
/*
* round lowpc and highpc to multiples of the density we're using
* so the rest of the scaling (here and in gprof) stays in ints.
@ -463,9 +470,14 @@ _mcleanup (void)
{
__moncontrol (0);
if (_gmonparam.state != GMON_PROF_ERROR)
if (_gmonparam.state != GMON_PROF_ERROR && _gmonparam.tos != NULL)
write_gmon ();
/* free the memory. */
free (_gmonparam.tos);
/* reset buffer to initial state for safety */
memset(&_gmonparam, 0, sizeof _gmonparam);
/* somewhat confusingly, ON=0, OFF=3 */
_gmonparam.state = GMON_PROF_OFF;
}

31
gmon/tst-mcleanup.c Normal file
View File

@ -0,0 +1,31 @@
/* Test program for repeated invocation of _mcleanup
Copyright The GNU Toolchain Authors.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
modify it under the terms of the GNU Lesser General Public
License as published by the Free Software Foundation; either
version 2.1 of the License, or (at your option) any later version.
The GNU C Library is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
Lesser General Public License for more details.
You should have received a copy of the GNU Lesser General Public
License along with the GNU C Library; if not, see
<https://www.gnu.org/licenses/>. */
/* Intentionally calls _mcleanup() twice: once manually, it will be
called again as an atexit handler. This is incorrect use of the API,
but the point of the test is to make sure we don't crash when the
API is misused in this way. */
#include <sys/gmon.h>
int
main (void)
{
_mcleanup();
return 0;
}