classification
Title: Use-after-free crash if multiple interpreters import asyncio module
Type: crash Stage: resolved
Components: C API Versions: Python 3.9, Python 3.8
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: jquesnelle, miss-islington, shihai1991, vstinner
Priority: normal Keywords: patch

Created on 2020-04-15 17:32 by jquesnelle, last changed 2020-04-17 02:42 by vstinner. This issue is now closed.

Files
File name Uploaded Description Edit
main.c jquesnelle, 2020-04-15 17:32 Reproduction C API program
Pull Requests
URL Status Linked Edit
PR 19542 merged jquesnelle, 2020-04-15 17:45
PR 19565 merged miss-islington, 2020-04-17 02:10
Messages (9)
msg366531 - (view) Author: Jeffrey Quesnelle (jquesnelle) * Date: 2020-04-15 17:32
Starting with Python 3.8 (GH-16598), the `_asyncio` module's C initialization is guarded behind a static variable. If the module is initialized a second time and this variable is set, the resources from the first initialization are used. However, when the module is freed and the corresponding resources released, the static variable is not cleared. If the module is subsequently initialized again, it will incorrectly believe it has already been initialized and use the previously freed resources, resulting in a crash.

This scenario is actually fairly easy to encounter in the presence of multiple interpreters whose lifetime is shorter than that of the whole program. Essentially, if any interpreter loads `asyncio` and then is freed with `Py_EndInterpreter`, any new interpreter that loads `asyncio` will crash. Since `asyncio` is a built-in module, it is loaded as a consequence of a wide variety of libraries.

I ran into this in my project because I use multiple interpreters to isolate user scripts, and I started to encounter crashes when switching to Python 3.8.

I've attached a simple reproduction program. I've personally tested that this runs without crashing in 3.6 and 3.7 (but I suspect it works down to 3.4 when `asyncio` was introduced).
msg366532 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-04-15 17:51
_asyncio should be ported to multiphase initialization (PEP 489) and its types converted to PyType_FromSpec() rather than using statically allocated types. See bpo-1635741.
msg366598 - (view) Author: Jeffrey Quesnelle (jquesnelle) * Date: 2020-04-16 14:08
Would the simple fix (clearing the flag in `module_free`) be a candidate for a backport to 3.8? This seems to be a regression from the previous stable version that also limits the usability of subinterpreters -- `asyncio` is loaded by a wide variety of libraries and so in general it's not easy to know that a particular subinterpreter hasn't loaded `asyncio`.   However, I concede that subinterpreters with variable lifetimes isn't a common use case.
msg366636 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-04-17 02:09
New changeset a75e730075cd25be1143e6183006f3b1d61bb80f by Jeffrey Quesnelle in branch 'master':
bpo-40294: Fix _asyncio when module is loaded/unloaded multiple times (GH-19542)
https://github.com/python/cpython/commit/a75e730075cd25be1143e6183006f3b1d61bb80f
msg366637 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-04-17 02:12
> Would the simple fix (clearing the flag in `module_free`) be a candidate for a backport to 3.8?

Sure.
msg366638 - (view) Author: miss-islington (miss-islington) Date: 2020-04-17 02:30
New changeset 6b0ca0aeab04d7b7b54086248ca9d5e70f770f2f by Miss Islington (bot) in branch '3.8':
bpo-40294: Fix _asyncio when module is loaded/unloaded multiple times (GH-19542)
https://github.com/python/cpython/commit/6b0ca0aeab04d7b7b54086248ca9d5e70f770f2f
msg366639 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-04-17 02:32
Is Python 3.7 affected as well?
msg366640 - (view) Author: Jeffrey Quesnelle (jquesnelle) * Date: 2020-04-17 02:37
> Is Python 3.7 affected as well?

Nope, this was introduced in 3.8
msg366641 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-04-17 02:42
Ok, the issue is now fixed: thanks Jeffrey Quesnelle!

> Nope, this was introduced in 3.8

I tested: attached main.c doesn't crash in 3.7. I guess that _asyncio leaks a few references, but it's not a big deal.
History
Date User Action Args
2020-06-22 21:27:42vstinnerlinkissue40965 superseder
2020-04-28 12:44:07vstinnerlinkissue40415 superseder
2020-04-17 02:42:48vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg366641

stage: patch review -> resolved
2020-04-17 02:37:33jquesnellesetmessages: + msg366640
2020-04-17 02:32:42vstinnersetmessages: + msg366639
2020-04-17 02:30:00miss-islingtonsetmessages: + msg366638
2020-04-17 02:12:08vstinnersetmessages: + msg366637
2020-04-17 02:10:05miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request18907
2020-04-17 02:09:49vstinnersetmessages: + msg366636
2020-04-16 14:11:22shihai1991setnosy: + shihai1991
2020-04-16 14:08:41jquesnellesetmessages: + msg366598
2020-04-15 17:51:06vstinnersetnosy: + vstinner
messages: + msg366532
2020-04-15 17:45:37jquesnellesetkeywords: + patch
stage: patch review
pull_requests: + pull_request18890
2020-04-15 17:32:05jquesnellecreate