Skip to content
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

Use-after-free crash if multiple interpreters import asyncio module #84474

Closed
jquesnelle mannequin opened this issue Apr 15, 2020 · 9 comments
Closed

Use-after-free crash if multiple interpreters import asyncio module #84474

jquesnelle mannequin opened this issue Apr 15, 2020 · 9 comments
Labels
3.8 only security fixes 3.9 only security fixes topic-C-API type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@jquesnelle
Copy link
Mannequin

jquesnelle mannequin commented Apr 15, 2020

BPO 40294
Nosy @vstinner, @miss-islington, @shihai1991, @jquesnelle
PRs
  • bpo-40294: Fix use-after-free in _asynciomodule.c when module is loaded/unloaded multiple times #19542
  • [3.8] bpo-40294: Fix _asyncio when module is loaded/unloaded multiple times (GH-19542) #19565
  • Files
  • main.c: Reproduction C API program
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = <Date 2020-04-17.02:42:48.843>
    created_at = <Date 2020-04-15.17:32:05.638>
    labels = ['expert-C-API', '3.8', '3.9', 'type-crash']
    title = 'Use-after-free crash if multiple interpreters import asyncio module'
    updated_at = <Date 2020-04-17.02:42:48.842>
    user = 'https://github.com/jquesnelle'

    bugs.python.org fields:

    activity = <Date 2020-04-17.02:42:48.842>
    actor = 'vstinner'
    assignee = 'none'
    closed = True
    closed_date = <Date 2020-04-17.02:42:48.843>
    closer = 'vstinner'
    components = ['C API']
    creation = <Date 2020-04-15.17:32:05.638>
    creator = 'jquesnelle'
    dependencies = []
    files = ['49064']
    hgrepos = []
    issue_num = 40294
    keywords = ['patch']
    message_count = 9.0
    messages = ['366531', '366532', '366598', '366636', '366637', '366638', '366639', '366640', '366641']
    nosy_count = 4.0
    nosy_names = ['vstinner', 'miss-islington', 'shihai1991', 'jquesnelle']
    pr_nums = ['19542', '19565']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue40294'
    versions = ['Python 3.8', 'Python 3.9']

    @jquesnelle
    Copy link
    Mannequin Author

    jquesnelle mannequin commented Apr 15, 2020

    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).

    @jquesnelle jquesnelle mannequin added 3.8 only security fixes 3.9 only security fixes topic-C-API type-crash A hard crash of the interpreter, possibly with a core dump labels Apr 15, 2020
    @vstinner
    Copy link
    Member

    _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.

    @jquesnelle
    Copy link
    Mannequin Author

    jquesnelle mannequin commented Apr 16, 2020

    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.

    @vstinner
    Copy link
    Member

    New changeset a75e730 by Jeffrey Quesnelle in branch 'master':
    bpo-40294: Fix _asyncio when module is loaded/unloaded multiple times (GH-19542)
    a75e730

    @vstinner
    Copy link
    Member

    Would the simple fix (clearing the flag in module_free) be a candidate for a backport to 3.8?

    Sure.

    @miss-islington
    Copy link
    Contributor

    New changeset 6b0ca0a by Miss Islington (bot) in branch '3.8':
    bpo-40294: Fix _asyncio when module is loaded/unloaded multiple times (GH-19542)
    6b0ca0a

    @vstinner
    Copy link
    Member

    Is Python 3.7 affected as well?

    @jquesnelle
    Copy link
    Mannequin Author

    jquesnelle mannequin commented Apr 17, 2020

    Is Python 3.7 affected as well?

    Nope, this was introduced in 3.8

    @vstinner
    Copy link
    Member

    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.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.8 only security fixes 3.9 only security fixes topic-C-API type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants