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

Py_FrozenMain() resource leak and missing malloc checks #60097

Closed
tiran opened this issue Sep 9, 2012 · 9 comments
Closed

Py_FrozenMain() resource leak and missing malloc checks #60097

tiran opened this issue Sep 9, 2012 · 9 comments
Assignees
Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@tiran
Copy link
Member

tiran commented Sep 9, 2012

BPO 15893
Nosy @jcea, @vstinner, @tiran
Files
  • issue-15893-01.patch
  • 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 = 'https://github.com/vstinner'
    closed_at = <Date 2014-03-23.10:00:53.818>
    created_at = <Date 2012-09-09.22:09:57.900>
    labels = ['interpreter-core', 'type-bug']
    title = 'Py_FrozenMain() resource leak and missing malloc checks'
    updated_at = <Date 2014-03-23.10:00:53.816>
    user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2014-03-23.10:00:53.816>
    actor = 'vstinner'
    assignee = 'vstinner'
    closed = True
    closed_date = <Date 2014-03-23.10:00:53.818>
    closer = 'vstinner'
    components = ['Interpreter Core']
    creation = <Date 2012-09-09.22:09:57.900>
    creator = 'christian.heimes'
    dependencies = []
    files = ['27202']
    hgrepos = []
    issue_num = 15893
    keywords = ['patch']
    message_count = 9.0
    messages = ['170139', '170547', '193763', '193767', '193768', '193769', '200725', '214535', '214558']
    nosy_count = 6.0
    nosy_names = ['jcea', 'vstinner', 'christian.heimes', 'thomaslee', 'BreamoreBoy', 'python-dev']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'commit review'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue15893'
    versions = ['Python 2.7', 'Python 3.2', 'Python 3.3', 'Python 3.4']

    @tiran
    Copy link
    Member Author

    tiran commented Sep 9, 2012

    In Python/frozenmain.c the function Py_FrozenMain() doesn't handle argv_copy and argv_copy2 correctly. Both variables contain memory that is allocated with PyMem_Malloc(). argv_copy2 is never checked for NULL and both variables are not correctly cleaned up in error cases.

    CID 486834: Resource leak (RESOURCE_LEAK)At (11): Variable "argv_copy" going out of scope leaks the storage it points to.
    CID 486835: Resource leak (RESOURCE_LEAK)At (9): Variable "argv_copy2" going out of scope leaks the storage it points to.

    Suggested fix:
    Separate var declaration from PyMem_Malloc() calls and use a goto label to clean up the variables and its content at the end of the function.

    @tiran tiran added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error labels Sep 9, 2012
    @thomaslee
    Copy link
    Mannequin

    thomaslee mannequin commented Sep 16, 2012

    Patch against hg tip attached.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 26, 2013

    New changeset ab8121466785 by Victor Stinner in branch '3.3':
    Issue bpo-15893: frozenmain.c now handles PyMem_Malloc() failure
    http://hg.python.org/cpython/rev/ab8121466785

    New changeset 386ab2c12301 by Victor Stinner in branch 'default':
    (Merge 3.3) Issue bpo-15893: frozenmain.c now handles PyMem_Malloc() failure
    http://hg.python.org/cpython/rev/386ab2c12301

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 27, 2013

    New changeset 47c6aa17fd90 by Victor Stinner in branch 'default':
    Issue bpo-15893: Improve error handling in main() and Py_FrozenMain()
    http://hg.python.org/cpython/rev/47c6aa17fd90

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 27, 2013

    New changeset 12af9db5212a by Victor Stinner in branch '3.3':
    Issue bpo-15893: Remove dead code
    http://hg.python.org/cpython/rev/12af9db5212a

    @vstinner
    Copy link
    Member

    I didn't know Py_FrozenMain(). I upgraded it to use the same code than main().

    Should I backport my fixes to Python 3.3 (except maybe 0001c4100823 which is risky).

    @tiran
    Copy link
    Member Author

    tiran commented Oct 21, 2013

    Victor, is here anything left to do?

    @tiran tiran added the stale Stale PR or inactive for long period of time. label Oct 21, 2013
    @BreamoreBoy
    Copy link
    Mannequin

    BreamoreBoy mannequin commented Mar 23, 2014

    This believe that this can be closed as Python 3.3 is no longer taking fixes.

    @vstinner
    Copy link
    Member

    Victor, is here anything left to do?

    The bug is correctly fixed in default. I don't really care of fixing such warning of static analyzer in older Python versions. It's more a theorical bug, it's a small memory leak and only occur if another error occurs. I just close the issue.

    @vstinner vstinner removed the stale Stale PR or inactive for long period of time. label Mar 23, 2014
    @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
    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

    2 participants