Skip to content

Commit 44ab80b

Browse files
committed
gh-156946: Unlink a curses panel before dropping its user pointer
PyCursesPanel_Dealloc() dropped the panel's user pointer first, through PyCursesPanel_Clear(), and only then called del_panel() and remove_lop(). Dropping that reference can run a __del__, and until del_panel() has run the dying panel is still on the panel stack and still in lop, so top_panel(), bottom_panel(), above() and below() hand the finalizer a new reference to an object whose refcount is already zero. Releasing that reference re-enters the deallocator and the interpreter segfaults. Take the panel out of lop and off the panel stack first, and release the user pointer and the window afterwards. set_panel_userptr() and panel_userptr() need the PANEL, so they still run before del_panel(). remove_lop() now also runs before del_panel(), which closes a second window where the registry held an entry whose PANEL had been freed. The deallocator cannot simply call PyCursesPanel_Clear() in the new order because that function is also tp_clear, where del_panel() must not run.
1 parent 1a2e3a0 commit 44ab80b

3 files changed

Lines changed: 28 additions & 8 deletions

File tree

Lib/test/test_curses.py

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2428,6 +2428,22 @@ def __del__(self):
24282428
panel.set_userptr(A())
24292429
panel.set_userptr(None)
24302430

2431+
@requires_curses_func('panel')
2432+
def test_userptr_dealloc_segfault(self):
2433+
w = curses.newwin(10, 10)
2434+
panel = curses.panel.new_panel(w)
2435+
seen = []
2436+
class A:
2437+
def __del__(self):
2438+
# The panel is being deallocated, so it must already be off
2439+
# the stack: handing it back here would resurrect an object
2440+
# whose refcount is zero -- segfaults.
2441+
seen.append(curses.panel.top_panel() is None)
2442+
panel.set_userptr(A())
2443+
del panel
2444+
gc_collect()
2445+
self.assertEqual(seen, [True])
2446+
24312447
@cpython_only
24322448
@requires_curses_func('panel')
24332449
def test_disallow_instantiation(self):
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a crash in :mod:`curses.panel` when the finalizer of a panel's user
2+
pointer runs while the panel is being deallocated. The panel is now taken
3+
off the panel stack before its user pointer is dropped.

Modules/_curses_panel.c

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -454,20 +454,21 @@ PyCursesPanel_Dealloc(PyObject *self)
454454
PyObject_GC_UnTrack(self);
455455

456456
PyCursesPanelObject *po = _PyCursesPanelObject_CAST(self);
457-
if (PyCursesPanel_Clear(self) < 0) {
457+
PyObject *extra = (PyObject *)panel_userptr(po->pan);
458+
if (extra != NULL && set_panel_userptr(po->pan, NULL) == ERR) {
459+
curses_panel_panel_set_error(po, "set_panel_userptr", "__del__");
460+
PyErr_FormatUnraisable("Exception ignored in PyCursesPanel_Dealloc()");
461+
}
462+
if (po->wo != NULL && remove_lop(po) < 0) {
463+
PyErr_SetString(PyExc_RuntimeError, "__del__: no panel object to delete");
458464
PyErr_FormatUnraisable("Exception ignored in PyCursesPanel_Dealloc()");
459465
}
460466
if (del_panel(po->pan) == ERR && !PyErr_Occurred()) {
461467
curses_panel_panel_set_error(po, "del_panel", "__del__");
462468
PyErr_FormatUnraisable("Exception ignored in PyCursesPanel_Dealloc()");
463469
}
464-
if (po->wo != NULL) {
465-
Py_DECREF(po->wo);
466-
if (remove_lop(po) < 0) {
467-
PyErr_SetString(PyExc_RuntimeError, "__del__: no panel object to delete");
468-
PyErr_FormatUnraisable("Exception ignored in PyCursesPanel_Dealloc()");
469-
}
470-
}
470+
Py_XDECREF(extra);
471+
Py_XDECREF(po->wo);
471472
tp->tp_free(po);
472473
Py_DECREF(tp);
473474
}

0 commit comments

Comments
 (0)