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

Partial moving of core objects to interpreter state is incorrect at best, unsafe at worse. #89854

Closed
markshannon opened this issue Nov 2, 2021 · 8 comments
Labels
3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@markshannon
Copy link
Member

BPO 45691
Nosy @vstinner, @markshannon, @ericsnowcurrently, @shihai1991, @erlend-aasland
PRs
  • bpo-45691: Make array of small ints static to fix use-after-free error. #29366
  • 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 = None
    created_at = <Date 2021-11-02.16:04:49.885>
    labels = ['interpreter-core', 'type-bug', '3.11']
    title = 'Partial moving of core objects to interpreter state is incorrect at best, unsafe at worse.'
    updated_at = <Date 2022-01-18.12:45:01.315>
    user = 'https://github.com/markshannon'

    bugs.python.org fields:

    activity = <Date 2022-01-18.12:45:01.315>
    actor = 'shihai1991'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Interpreter Core']
    creation = <Date 2021-11-02.16:04:49.885>
    creator = 'Mark.Shannon'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 45691
    keywords = ['patch']
    message_count = 5.0
    messages = ['405514', '405628', '406483', '410811', '410812']
    nosy_count = 5.0
    nosy_names = ['vstinner', 'Mark.Shannon', 'eric.snow', 'shihai1991', 'erlendaasland']
    pr_nums = ['29366']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue45691'
    versions = ['Python 3.11']

    @markshannon
    Copy link
    Member Author

    We currently have an unstable state in the VM where some core objects are static and some are per-interpreter.

    For example, smalls ints are allocated per-interpreter, but many classes are allocated statically.
    This means that if any int is reachable from a class, then references to per-interpreter objects can be left dangling, or be out of date.

    E.g. consider this sequence:

    1. Create an interpreter
    2. Destroy it.
    3. Create a new interpreter

    sys.float_info.n_unnamed_fields causes a memory violation if the per-interpreter allocated 0 held by sys.float_info.n_unnamed_fields is freed.
    If it is not freed, then sys.float_info.n_unnamed_fields is 0 is False, meaning that there are two zeros present.

    The above is just an example. Classes have many references to ints, floats, code objects, etc. Any of those could have the same issue.

    All objects that form the core object graph must either be entirely static, or entirely per-interpreter.

    We cannot change from static to per-interpreter in a piecemeal fashion. It must be done all at once.

    @markshannon markshannon added 3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Nov 2, 2021
    @markshannon
    Copy link
    Member Author

    New changeset acc89db by Mark Shannon in branch 'main':
    bpo-45691: Make array of small ints static to fix use-after-free error. (GH-29366)
    acc89db

    @vstinner
    Copy link
    Member

    many classes are allocated statically

    Right. Changing that is an hard problem :-( See for example bpo-40601 "[C API] Hide static types from the limited C API".

    I tried once to "free" / reset static types in Py_Finalize(), but it's hard to implement properly :-(

    @vstinner
    Copy link
    Member

    Mark:

    sys.float_info.n_unnamed_fields causes a memory violation if the per-interpreter allocated 0 held by sys.float_info.n_unnamed_fields is freed.

    I created bpo-46417 follow-up issue: "[subinterpreters] Clear static types in Py_Finalize()".

    @vstinner
    Copy link
    Member

    sys.float_info.n_unnamed_fields causes a memory violation if the per-interpreter allocated 0 held by sys.float_info.n_unnamed_fields is freed.
    If it is not freed, then sys.float_info.n_unnamed_fields is 0 is False, meaning that there are two zeros present.

    Python 3.9 and 3.10 are concerned by this issue: integer singletons are per-interpreter since Python 3.9.

    Should Python 3.9 and 3.10 be fixed? "x is 0" is recommended and should be used. For example, the compiler emits a SyntaxWarning:

    $ python3.9
    >>> x=0
    >>> x is 0
    <stdin>:1: SyntaxWarning: "is" with a literal. Did you mean "=="?
    True

    I propose to only fix the main branch and so close the issue.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    @gvanrossum
    Copy link
    Member

    This wasn't closed. Is fixing this still targeted for 3.11? In that case it should probably be made a release blocker. If not, the label should be changed to 3.12. @vstinner @markshannon @ericsnowcurrently

    @vstinner
    Copy link
    Member

    vstinner commented Jul 3, 2022

    This wasn't closed.

    @markshannon fixed the issue with acc89db

    This issue can now be closed, no? Or is there a remaining issue?

    I tried once to "free" / reset static types in Py_Finalize(), but it's hard to implement properly :-(

    I implemented that in issue #90575.

    @gvanrossum
    Copy link
    Member

    The array of small ints got moved again, it's now in pycore_global_objects.h. I will close this issue.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants