Skip to content

[3.13] gh-120289: Disallow disable() and clear() in external timer to prevent use-after-free (GH-120297) #121984

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions Lib/test/test_cprofile.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,43 @@ def test_bad_counter_during_dealloc(self):

self.assertEqual(cm.unraisable.exc_type, TypeError)

def test_evil_external_timer(self):
# gh-120289
# Disabling profiler in external timer should not crash
import _lsprof
class EvilTimer():
def __init__(self, disable_count):
self.count = 0
self.disable_count = disable_count

def __call__(self):
self.count += 1
if self.count == self.disable_count:
profiler_with_evil_timer.disable()
return self.count

# this will trigger external timer to disable profiler at
# call event - in initContext in _lsprof.c
with support.catch_unraisable_exception() as cm:
profiler_with_evil_timer = _lsprof.Profiler(EvilTimer(1))
profiler_with_evil_timer.enable()
# Make a call to trigger timer
(lambda: None)()
profiler_with_evil_timer.disable()
profiler_with_evil_timer.clear()
self.assertEqual(cm.unraisable.exc_type, RuntimeError)

# this will trigger external timer to disable profiler at
# return event - in Stop in _lsprof.c
with support.catch_unraisable_exception() as cm:
profiler_with_evil_timer = _lsprof.Profiler(EvilTimer(2))
profiler_with_evil_timer.enable()
# Make a call to trigger timer
(lambda: None)()
profiler_with_evil_timer.disable()
profiler_with_evil_timer.clear()
self.assertEqual(cm.unraisable.exc_type, RuntimeError)

def test_profile_enable_disable(self):
prof = self.profilerclass()
# Make sure we clean ourselves up if the test fails for some reason.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Fixed the use-after-free issue in :mod:`cProfile` by disallowing
``disable()`` and ``clear()`` in external timers.
20 changes: 19 additions & 1 deletion Modules/_lsprof.c
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ typedef struct {
#define POF_ENABLED 0x001
#define POF_SUBCALLS 0x002
#define POF_BUILTINS 0x004
#define POF_EXT_TIMER 0x008
#define POF_NOMEMORY 0x100

/*[clinic input]
Expand Down Expand Up @@ -87,7 +88,14 @@ _lsprof_get_state(PyObject *module)

static PyTime_t CallExternalTimer(ProfilerObject *pObj)
{
PyObject *o = _PyObject_CallNoArgs(pObj->externalTimer);
PyObject *o = NULL;

// External timer can do arbitrary things so we need a flag to prevent
// horrible things to happen
pObj->flags |= POF_EXT_TIMER;
o = _PyObject_CallNoArgs(pObj->externalTimer);
pObj->flags &= ~POF_EXT_TIMER;

if (o == NULL) {
PyErr_WriteUnraisable(pObj->externalTimer);
return 0;
Expand Down Expand Up @@ -777,6 +785,11 @@ Stop collecting profiling information.\n\
static PyObject*
profiler_disable(ProfilerObject *self, PyObject* noarg)
{
if (self->flags & POF_EXT_TIMER) {
PyErr_SetString(PyExc_RuntimeError,
"cannot disable profiler in external timer");
return NULL;
}
if (self->flags & POF_ENABLED) {
PyObject* result = NULL;
PyObject* monitoring = _PyImport_GetModuleAttrString("sys", "monitoring");
Expand Down Expand Up @@ -830,6 +843,11 @@ Clear all profiling information collected so far.\n\
static PyObject*
profiler_clear(ProfilerObject *pObj, PyObject* noarg)
{
if (pObj->flags & POF_EXT_TIMER) {
PyErr_SetString(PyExc_RuntimeError,
"cannot clear profiler in external timer");
return NULL;
}
clearEntries(pObj);
Py_RETURN_NONE;
}
Expand Down
Loading