gh-156946: Unlink a curses panel before dropping its user pointer

[fedonman]

PyCursesPanel_Dealloc() released the panel's user pointer before it took the panel off the panel stack and out of lop, so a __del__ running from that Py_DECREF could fetch the dying panel back through top_panel() and crash the interpreter.

The deallocator now unlinks first (set_panel_userptr(NULL), remove_lop(), del_panel()) and releases the user pointer and the window afterwards. remove_lop() also moves ahead of del_panel(), closing a second window where the registry held an entry whose PANEL had already been freed. It cannot just call PyCursesPanel_Clear() in the new order, because that function is also tp_clear, where del_panel() must not run.

The new test segfaults the worker without the change to Modules/_curses_panel.c, which is also true of the neighbouring test_userptr_segfault.

With the test but without the C change:

$ TERM=xterm-256color ./python -m test -u all -v test_curses -m test_userptr_dealloc_segfault; echo "rc=$?"
test_userptr_dealloc_segfault (test.test_curses.TestCurses.test_userptr_dealloc_segfault) ... Fatal Python error: Segmentation fault

Current thread 0x00007059a8a4e780 [python] (most recent call first):
  File "Lib/test/test_curses.py", line 2443 in test_userptr_dealloc_segfault
  ...
rc=139

With the change:

$ TERM=xterm-256color ./python -m test -u all -v test_curses -m test_userptr_dealloc_segfault
test_userptr_dealloc_segfault (test.test_curses.TestCurses.test_userptr_dealloc_segfault) ... ok
== Tests result: SUCCESS ==

Full module:

$ TERM=xterm-256color ./python -m test -u all test_curses
Total tests: run=184 skipped=3
Result: SUCCESS

• Issue: Segfault when a curses panel's userptr finalizer runs at deallocation #156946

[serhiy-storchaka]

There is a similar issue in PyCursesPanel_Clear(). Py_DECREF(extra) should be called after set_panel_userptr().

[fedonman]

Thanks, done.

[serhiy-storchaka]

LGTM. 👍

[miss-islington-app]

Thanks @fedonman for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14, 3.15.
🐍🍒⛏🤖

[miss-islington-app]

Sorry, @fedonman and @serhiy-storchaka, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker df02e264c8b42e0351b36e1c6e25c1e63b7a6a02 3.14

[miss-islington-app]

Sorry, @fedonman and @serhiy-storchaka, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker df02e264c8b42e0351b36e1c6e25c1e63b7a6a02 3.13

[bedevere-app]

GH-157540 is a backport of this pull request to the 3.15 branch.

[bedevere-app]

GH-157541 is a backport of this pull request to the 3.14 branch.

[bedevere-app]

GH-157542 is a backport of this pull request to the 3.13 branch.

[bedevere-bot]

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Arch Linux Asan 3.13 (tierless) has failed when building commit f63df37.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1464/builds/1264) and take a look at the build logs.
4. Check if the failure is related to this commit (f63df37) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1464/builds/1264

Failed tests:

• test_io

Failed subtests:

• test_daemon_threads_shutdown_stderr_deadlock - test.test_io.CMiscIOTest.test_daemon_threads_shutdown_stderr_deadlock
• test_daemon_threads_shutdown_stdout_deadlock - test.test_io.CMiscIOTest.test_daemon_threads_shutdown_stdout_deadlock

Summary of the results of the build (if available):

==

Click to see traceback logs

Traceback (most recent call last):
  File "/buildbot/buildarea/3.13.pablogsal-arch-x86_64.asan/build/Lib/test/test_io.py", line 4816, in test_daemon_threads_shutdown_stdout_deadlock
    self.check_daemon_threads_shutdown_deadlock('stdout')
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^
  File "/buildbot/buildarea/3.13.pablogsal-arch-x86_64.asan/build/Lib/test/test_io.py", line 4811, in check_daemon_threads_shutdown_deadlock
    self.assertFalse(err.strip('.!'))
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
AssertionError: '=================================================================\n==2720307==ERROR: AddressSanitizer: heap-use-after-free on address 0x7ca91f252898 at pc 0x55f65a4542c6 bp 0x7b791cffe080 sp 0x7b791cffe070\nREAD of size 8 at 0x7ca91f252898 thread T1\n' is not false


Traceback (most recent call last):
  File "/buildbot/buildarea/3.13.pablogsal-arch-x86_64.asan/build/Lib/test/test_io.py", line 4821, in test_daemon_threads_shutdown_stderr_deadlock
    self.check_daemon_threads_shutdown_deadlock('stderr')
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^
  File "/buildbot/buildarea/3.13.pablogsal-arch-x86_64.asan/build/Lib/test/test_io.py", line 4811, in check_daemon_threads_shutdown_deadlock
    self.assertFalse(err.strip('.!'))
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
AssertionError: '=================================================================\n==2716240==ERROR: AddressSanitizer: heap-use-after-free on address 0x7c9663052898 at pc 0x55774d6702c6 bp 0x7b6660dfe080 sp 0x7b6660dfe070\nREAD of size 8 at 0x7c9663052898 thread T1\n' is not false


Traceback (most recent call last):
  File "/buildbot/buildarea/3.13.pablogsal-arch-x86_64.asan/build/Lib/test/test_io.py", line 4816, in test_daemon_threads_shutdown_stdout_deadlock
    self.check_daemon_threads_shutdown_deadlock('stdout')
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^
  File "/buildbot/buildarea/3.13.pablogsal-arch-x86_64.asan/build/Lib/test/test_io.py", line 4811, in check_daemon_threads_shutdown_deadlock
    self.assertFalse(err.strip('.!'))
    ~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
AssertionError: '=================================================================\n==2716245==ERROR: AddressSanitizer: heap-use-after-free on address 0x7c40f7452898 at pc 0x564e3292d2c6 bp 0x7b10f51fe080 sp 0x7b10f51fe070\nREAD of size 8 at 0x7c40f7452898 thread T1\n' is not false

[bedevere-bot]

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 CentOS9 Refleaks 3.x (tier-1) has failed when building commit df02e26.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/853/builds/4257) and take a look at the build logs.
4. Check if the failure is related to this commit (df02e26) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/853/builds/4257

Summary of the results of the build (if available):

==

Click to see traceback logs

Note: switching to 'df02e264c8b42e0351b36e1c6e25c1e63b7a6a02'.

You are in 'detached HEAD' state. You can look around, make experimental
changes and commit them, and you can discard any commits you make in this
state without impacting any branches by switching back to a branch.

If you want to create a new branch to retain commits you create, you may
do so (now or later) by using -c with the switch command. Example:

  git switch -c <new-branch-name>

Or undo this operation with:

  git switch -

Turn off this advice by setting config variable advice.detachedHead to false

HEAD is now at df02e264c8b gh-156946: Unlink a curses panel before dropping its user pointer (GH-156947)
Switched to and reset branch 'main'

make: *** [Makefile:2481: buildbottest] Error 3

Expanding reachable commits in commit graph: 92289
Expanding reachable commits in commit graph: 171166, done.