bpo-45262: Prevent use-after-free in asyncio by HiassofT · Pull Request #28796 · python/cpython

HiassofT · 2021-10-07T13:28:32Z

Make sure the cached running loop holder gets cleared on dealloc to prevent use-after-free in get_running_loop

https://bugs.python.org/issue45262

the-knights-who-say-ni · 2021-10-07T13:28:47Z

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

CLA Missing

Our records indicate the following people have not signed the CLA:

@HiassofT

For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

If you have recently signed the CLA, please wait at least one business day
before our records are updated.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

Modules/_asynciomodule.c

vstinner · 2021-10-07T13:55:37Z

Can you please try to add a NEWS entry to document your bugfix? See https://devguide.python.org/

HiassofT · 2021-10-07T14:09:07Z

thanks a lot for walking me through, I've added the News entry

vstinner

LGTM.

miss-islington · 2021-10-07T22:47:08Z

Thanks @HiassofT for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

miss-islington · 2021-10-07T22:47:09Z

Thanks @HiassofT for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

miss-islington · 2021-10-07T22:47:10Z

Sorry @HiassofT and @vstinner, I had trouble checking out the 3.10 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 392a89835371baa0fc4bf79ae479abb80661f57d 3.10

bedevere-bot · 2021-10-07T22:47:19Z

GH-28815 is a backport of this pull request to the 3.9 branch.

) Prevent use-after-free of running loop holder via cache. (cherry picked from commit 392a898) Co-authored-by: Matthias Reichl <github@hias.horus.com>

vstinner · 2021-10-07T22:47:27Z

@HiassofT: Oh, I didn't notice that you didn't sign the CLA. Please sign it ;-)

miss-islington · 2021-10-07T22:47:38Z

Thanks @HiassofT for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10.
🐍🍒⛏🤖

bedevere-bot · 2021-10-07T22:47:47Z

GH-28816 is a backport of this pull request to the 3.10 branch.

) Prevent use-after-free of running loop holder via cache. (cherry picked from commit 392a898) Co-authored-by: Matthias Reichl <github@hias.horus.com>

Prevent use-after-free of running loop holder via cache. (cherry picked from commit 392a898) Co-authored-by: Matthias Reichl <github@hias.horus.com>

…H-28816) Prevent use-after-free of running loop holder via cache. (cherry picked from commit 392a898) Co-authored-by: Matthias Reichl <github@hias.horus.com>

HiassofT · 2021-10-08T09:04:05Z

@vstinner CLA seems to have been delayed in approval-queue, but it looks all good now

bedevere-bot · 2021-10-08T09:04:17Z

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

Hi! The buildbot aarch64 RHEL8 3.10 has failed when building commit 6846d67.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/612/builds/380) and take a look at the build logs.
Check if the failure is related to this commit (6846d67) or if it is a false positive.
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/all/#builders/612/builds/380

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

== Tests result: ENV CHANGED ==

414 tests OK.

10 slowest tests:

test_gdb: 5 min 21 sec
test_concurrent_futures: 3 min 11 sec
test_multiprocessing_spawn: 2 min 35 sec
test_peg_generator: 2 min 27 sec
test_asyncio: 2 min 8 sec
test_unparse: 1 min 40 sec
test_multiprocessing_forkserver: 1 min 31 sec
test_multiprocessing_fork: 1 min 18 sec
test_nntplib: 1 min 10 sec
test_capi: 1 min 5 sec

1 test altered the execution environment:
test_ftplib

12 tests skipped:
test_devpoll test_ioctl test_kqueue test_msilib test_startfile
test_tix test_tk test_ttk_guionly test_winconsoleio test_winreg
test_winsound test_zipfile64

Total duration: 5 min 44 sec

Click to see traceback logs

Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.10.cstratak-RHEL8-aarch64/build/Lib/multiprocessing/resource_tracker.py", line 209, in main
    cache[rtype].remove(name)
KeyError: '/psm_ba48173f'


Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.10.cstratak-RHEL8-aarch64/build/Lib/multiprocessing/resource_tracker.py", line 209, in main
    cache[rtype].remove(name)
KeyError: '/psm_520b2476'


Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.10.cstratak-RHEL8-aarch64/build/Lib/multiprocessing/resource_tracker.py", line 209, in main
    cache[rtype].remove(name)
KeyError: '/psm_1de96852'


Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.10.cstratak-RHEL8-aarch64/build/Lib/asyncore.py", line 90, in read
    obj.handle_read_event()
  File "/home/buildbot/buildarea/3.10.cstratak-RHEL8-aarch64/build/Lib/test/test_ftplib.py", line 388, in handle_read_event
    super(SSLConnection, self).handle_read_event()
  File "/home/buildbot/buildarea/3.10.cstratak-RHEL8-aarch64/build/Lib/asyncore.py", line 427, in handle_read_event
    self.handle_read()
  File "/home/buildbot/buildarea/3.10.cstratak-RHEL8-aarch64/build/Lib/asynchat.py", line 179, in handle_read
    self.found_terminator()
  File "/home/buildbot/buildarea/3.10.cstratak-RHEL8-aarch64/build/Lib/test/test_ftplib.py", line 135, in found_terminator
    method(arg)
  File "/home/buildbot/buildarea/3.10.cstratak-RHEL8-aarch64/build/Lib/test/test_ftplib.py", line 160, in cmd_pasv
    conn, addr = sock.accept()
  File "/home/buildbot/buildarea/3.10.cstratak-RHEL8-aarch64/build/Lib/socket.py", line 293, in accept
    fd, addr = self._accept()
TimeoutError: timed out


Traceback (most recent call last):
  File "/home/buildbot/buildarea/3.10.cstratak-RHEL8-aarch64/build/Lib/threading.py", line 1009, in _bootstrap_inner
    self.run()
  File "/home/buildbot/buildarea/3.10.cstratak-RHEL8-aarch64/build/Lib/test/test_ftplib.py", line 298, in run
    asyncore.loop(timeout=0.1, count=1)
  File "/home/buildbot/buildarea/3.10.cstratak-RHEL8-aarch64/build/Lib/asyncore.py", line 214, in loop
    poll_fun(timeout, map)
  File "/home/buildbot/buildarea/3.10.cstratak-RHEL8-aarch64/build/Lib/asyncore.py", line 157, in poll
    read(obj)
  File "/home/buildbot/buildarea/3.10.cstratak-RHEL8-aarch64/build/Lib/asyncore.py", line 94, in read
    obj.handle_error()
  File "/home/buildbot/buildarea/3.10.cstratak-RHEL8-aarch64/build/Lib/test/test_ftplib.py", line 421, in handle_error
    raise Exception
Exception

vstinner · 2021-10-08T09:25:24Z

@vstinner CLA seems to have been delayed in approval-queue, but it looks all good now

Thank you :-)

HiassofT requested review from 1st1 and asvetlov as code owners October 7, 2021 13:28

bedevere-bot added the awaiting review label Oct 7, 2021

the-knights-who-say-ni added the CLA not signed label Oct 7, 2021

vstinner reviewed Oct 7, 2021

View reviewed changes

Modules/_asynciomodule.c Outdated Show resolved Hide resolved

HiassofT force-pushed the fix-running-holder branch from f1d72b8 to c4e2aa8 Compare October 7, 2021 13:48

Prevent use-after-free of running loop holder via cache

9289edb

HiassofT force-pushed the fix-running-holder branch from d94896b to 9289edb Compare October 7, 2021 14:06

vstinner approved these changes Oct 7, 2021

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Oct 7, 2021

vstinner merged commit 392a898 into python:main Oct 7, 2021

bedevere-bot removed the awaiting merge label Oct 7, 2021

vstinner added needs backport to 3.9 needs backport to 3.10 only security fixes labels Oct 7, 2021

miss-islington assigned vstinner Oct 7, 2021

bedevere-bot removed the needs backport to 3.9 label Oct 7, 2021

vstinner removed CLA not signed needs backport to 3.10 only security fixes labels Oct 7, 2021

the-knights-who-say-ni added the CLA signed label Oct 7, 2021

vstinner removed the CLA signed label Oct 7, 2021

vstinner added the needs backport to 3.10 only security fixes label Oct 7, 2021

the-knights-who-say-ni added the CLA signed label Oct 7, 2021

bedevere-bot removed the needs backport to 3.10 only security fixes label Oct 7, 2021

benjamin-sch mannequin mentioned this pull request Apr 16, 2022

crash if asyncio is used before and after re-initialization if using python embedded in an application #89425

Closed

Uh oh!

Conversation

HiassofT commented Oct 7, 2021 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

the-knights-who-say-ni commented Oct 7, 2021

CLA Missing

Uh oh!

Uh oh!

vstinner commented Oct 7, 2021

Uh oh!

HiassofT commented Oct 7, 2021

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington commented Oct 7, 2021

Uh oh!

miss-islington commented Oct 7, 2021

Uh oh!

miss-islington commented Oct 7, 2021

Uh oh!

bedevere-bot commented Oct 7, 2021

Uh oh!

vstinner commented Oct 7, 2021

Uh oh!

miss-islington commented Oct 7, 2021

Uh oh!

bedevere-bot commented Oct 7, 2021

Uh oh!

HiassofT commented Oct 8, 2021

Uh oh!

bedevere-bot commented Oct 8, 2021

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

Uh oh!

vstinner commented Oct 8, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

HiassofT commented Oct 7, 2021 •

edited by bedevere-bot

Loading