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

Python could crash while creating weakref for a given object #73533

Closed
dhanavaths mannequin opened this issue Jan 23, 2017 · 19 comments
Closed

Python could crash while creating weakref for a given object #73533

dhanavaths mannequin opened this issue Jan 23, 2017 · 19 comments
Labels
3.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump

Comments

@dhanavaths
Copy link
Mannequin

dhanavaths mannequin commented Jan 23, 2017

BPO 29347
Nosy @freddrake, @tiran, @zhangyangyu, @dhanavaths
PRs
  • bpo-29347: Fix possibly dereferencing undefined pointers when creating weakref objects #128
  • bpo-29347: Fix possibly dereferencing undefined pointers when creating weakref objects #186
  • bpo-29347: Fix possibly dereferencing undefined pointers when creating weakref objects #187
  • bpo-29347: Fix possibly dereferencing undefined pointers when creating weakref objects #188
  • [Do Not Merge] Sample of CPython life with blurb. #703
  • [Do Not Merge] Convert Misc/NEWS so that it is managed by towncrier #552
  • Files
  • crash_in_weakref.zip
  • weakref_crash.patch
  • verify_crash_in_weakref.zip
  • weakref_crash.c
  • 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 2017-02-20.06:35:53.717>
    created_at = <Date 2017-01-23.03:24:01.147>
    labels = ['interpreter-core', '3.7', 'type-crash']
    title = 'Python could crash while creating weakref for a given object'
    updated_at = <Date 2017-03-31.16:36:36.126>
    user = 'https://github.com/dhanavaths'

    bugs.python.org fields:

    activity = <Date 2017-03-31.16:36:36.126>
    actor = 'dstufft'
    assignee = 'none'
    closed = True
    closed_date = <Date 2017-02-20.06:35:53.717>
    closer = 'xiang.zhang'
    components = ['Interpreter Core']
    creation = <Date 2017-01-23.03:24:01.147>
    creator = 'dhanavaths'
    dependencies = []
    files = ['46396', '46513', '46515', '46516']
    hgrepos = []
    issue_num = 29347
    keywords = ['patch']
    message_count = 19.0
    messages = ['286044', '286062', '286116', '286133', '286926', '286935', '286948', '286953', '286956', '286964', '286965', '287912', '288180', '288181', '288182', '288183', '288184', '288270', '288271']
    nosy_count = 4.0
    nosy_names = ['fdrake', 'christian.heimes', 'xiang.zhang', 'dhanavaths']
    pr_nums = ['128', '186', '187', '188', '703', '552']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'crash'
    url = 'https://bugs.python.org/issue29347'
    versions = ['Python 2.7', 'Python 3.5', 'Python 3.6', 'Python 3.7']

    @dhanavaths
    Copy link
    Mannequin Author

    dhanavaths mannequin commented Jan 23, 2017

    We are using python 2.7.8 on Ubuntu 14.04 to host our services. In one of the crashes python interpreter got segmentation fault while initializing weakref for a given object. Please find snip of backtraces as given below.

    #0 0x00007f62aa86951a in clear_weakref (self=0x7f5a1ed17520) at Objects/weakrefobject.c:65
    #1 proxy_dealloc (self=0x7f5a1ed17520) at Objects/weakrefobject.c:540
    #2 0x00007f62aa869b8b in PyWeakref_NewProxy (ob=<optimized out>, callback=<optimized out>) at Objects/weakrefobject.c:855
    #3 0x00007f62aa901e56 in weakref_proxy (self=<optimized out>, args=<optimized out>) at ./Modules/_weakref.c:73
    #4 0x00007f62aa8a929b in call_function (oparg=<optimized out>, pp_stack=0x7f5d36661c90) at Python/ceval.c:4033
    .
    .
    .

    Have tried to root cause the issue and found that PyWeakref_NewProxy@Objects/weakrefobject.c creates new isntance of PyWeakReference struct and does not intialize wr_prev and wr_next of new isntance. These pointers can have garbage and point to random memory locations.

    As per comment in the code there could be a race while creating new instance and some other thread could have created weakref by the time current thread returns from new_weakref function. If it finds weakref created, current thread destroys instance created by itself and uses the one created by some other thread.

    Python should not crash while destroying the isntance created in the same interpreter function. As per my understanding, both wr_prev and wr_next of PyWeakReference instance should be initialized to NULL to avoid segfault.

    @dhanavaths dhanavaths mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump labels Jan 23, 2017
    @tiran
    Copy link
    Member

    tiran commented Jan 23, 2017

    Can you reproduce the issue with a more recent version of Python 2.7? 2.7.8 is pretty old.

    @dhanavaths
    Copy link
    Mannequin Author

    dhanavaths mannequin commented Jan 23, 2017

    Hi Christian Heimes,

    PFA. I have written a some code to simulate and test PyWeakReference struct instantion and then hit segfault based on flag passed-in to C code. Here I am trying to execute some of the operations from new_weakref and dealloc_weakref of Objects/weakrefobject.c to show that new isntance of PyWeakReference is not initialized properly. Have also checked latest 3.6 source and there is no difference in alloc and dealloc routines of 2.7.8 and 3.6.0. Have run test code on 2.7.8, 2.7.12+, 3.4m and 3.5m interpreters and got segfault in all runs.

    @dhanavaths
    Copy link
    Mannequin Author

    dhanavaths mannequin commented Jan 24, 2017

    Hi Christian Heimes,

    Please ignore typos in the previous post. I have written some code to simulate and test PyWeakReference struct instantiation and then hit segfault based on the flag passed to C code. Here I am trying to execute some of the operations from new_weakref and dealloc_weakref of Objects/weakrefobject.c to show that new instance of PyWeakReference is not initialized properly and results in segfault. Have also checked python 3.6.0 source and I do not see any change in weakref alloc and dealloc routines of 2.7.8 and 3.6.0 versions. Have run test code on 2.7.8, 2.7.12+, 3.4m and 3.5m interpreters and got segfault in all runs.

    Please find the sample output as given below.

    ubuntu@ubuntu1610saida:~/weakref$ make build PYVERSION=2.7
    swig -python weakref_crash.i
    gcc -w -fpic -c weakref_crash.c weakref_crash_wrap.c -I/usr/include/python2.7
    gcc -shared weakref_crash.o weakref_crash_wrap.o -o _pyweakref_crash.so -L/usr/lib/python2.7 -lpython2.7

    ubuntu@ubuntu1610saida:~/weakref$ python test.py 0 0
    2.7.12+ (default, Sep 17 2016, 12:08:02)
    [GCC 6.2.0 20160914]
    0
    0

    ubuntu@ubuntu1610saida:~/weakref$ python test.py 0 1
    2.7.12+ (default, Sep 17 2016, 12:08:02)
    [GCC 6.2.0 20160914]
    0
    0

    ubuntu@ubuntu1610saida:~/weakref$ python test.py 1 0
    2.7.12+ (default, Sep 17 2016, 12:08:02)
    [GCC 6.2.0 20160914]
    1010101
    1010101

    ubuntu@ubuntu1610saida:~/weakref$ python test.py 1 1
    2.7.12+ (default, Sep 17 2016, 12:08:02)
    [GCC 6.2.0 20160914]
    1010101
    1010101
    Segmentation fault (core dumped)

    ubuntu@ubuntu1610saida:~/weakref$ make clean
    rm -f *.so *.o *crash.py *.pyc *crash_wrap.c

    ubuntu@ubuntu1610saida:/weakref$ make build PYVERSION=3.5m
    swig -python weakref_crash.i
    gcc -w -fpic -c weakref_crash.c weakref_crash_wrap.c -I/usr/include/python3.5m
    gcc -shared weakref_crash.o weakref_crash_wrap.o -o _pyweakref_crash.so -L/usr/lib/python3.5m -lpython3.5m
    ubuntu@ubuntu1610saida:
    /weakref$ python3.5m test.py 0 0
    3.5.2+ (default, Sep 22 2016, 12:18:14)
    [GCC 6.2.0 20160927]
    0
    0
    ubuntu@ubuntu1610saida:/weakref$ python3.5m test.py 0 1
    3.5.2+ (default, Sep 22 2016, 12:18:14)
    [GCC 6.2.0 20160927]
    0
    0
    ubuntu@ubuntu1610saida:
    /weakref$ python3.5m test.py 1 0
    3.5.2+ (default, Sep 22 2016, 12:18:14)
    [GCC 6.2.0 20160927]
    1010101
    101
    ubuntu@ubuntu1610saida:~/weakref$ python3.5m test.py 1 1
    3.5.2+ (default, Sep 22 2016, 12:18:14)
    [GCC 6.2.0 20160927]
    1010101
    101
    Segmentation fault (core dumped)

    @dhanavaths dhanavaths mannequin added the 3.7 (EOL) end of life label Feb 2, 2017
    @zhangyangyu
    Copy link
    Member

    After reading the code I could see the possibility. A weakref object gets two linkedlist pointers which are not initialized by new_weakref (actually they are initialized by insert_head or insert_after). But the weakref object is possible to be destroyed in [1] and [2]. So we are going to dereference two uninitialized pointers in clear_weakref and then crash. So simply initialize the two pointers to NULL in init_weakref could solve this problem? Are you willing to test Saida?

    [1] https://github.com/python/cpython/blob/master/Objects/weakrefobject.c#L770
    [2] https://github.com/python/cpython/blob/master/Objects/weakrefobject.c#L833

    @zhangyangyu zhangyangyu changed the title Python 2.7.8 is crashing while creating weakref for a given object. Python could crash while creating weakref for a given object Feb 4, 2017
    @dhanavaths
    Copy link
    Mannequin Author

    dhanavaths mannequin commented Feb 4, 2017

    Hi xiang,

    I have already patched our build environment with the fix and tested it locally. Although I cannot test it in production, have taken out the code of new_weakref, init_weakref and clear_weakref from weakrefobject.c. Changed init_weakref to initialize wr_prev and wr_next with NULL and verified that test program does crash after fix.

    @zhangyangyu
    Copy link
    Member

    Hmm, what's your test program? Would you mind show it?

    @dhanavaths
    Copy link
    Mannequin Author

    dhanavaths mannequin commented Feb 4, 2017

    Please find the test program attached. Readme.txt has steps to comiple and run program.

    @zhangyangyu
    Copy link
    Member

    But your weakref_crash.c doesn't look correct to me. Your test.object type doesn't support weak references at all. How could you use GET_WEAKREFS_LISTPTR then? See https://docs.python.org/3/extending/newtypes.html#weakref-support.

    @zhangyangyu
    Copy link
    Member

    Saida, I changed your test program to use set instead of self created type (see attachment). I tested it under Py2.7 and it seems no crash happens.

    python test.py 1 1
    2.7.10 (default, Oct 14 2015, 16:09:02)
    [GCC 5.2.1 20151010]
    0x101010101010101
    0x1010101010101
    Segmentation fault (core dumped)

    python test.py 1 1
    2.7.10 (default, Oct 14 2015, 16:09:02)
    [GCC 5.2.1 20151010]
    (nil)
    (nil)

    It's appreciated if you are willing to make more tests or even in your build environment. :-)

    @dhanavaths
    Copy link
    Mannequin Author

    dhanavaths mannequin commented Feb 4, 2017

    Xiang,

    Sure, I will run it with other python versions and post the results.

    @zhangyangyu
    Copy link
    Member

    Ping.

    @zhangyangyu
    Copy link
    Member

    New changeset d0e8212 by GitHub in branch 'master':
    bpo-29347: Fix possibly dereferencing undefined pointers when creating weakref objects (#128)
    d0e8212

    @zhangyangyu
    Copy link
    Member

    New changeset 7131a73 by GitHub in branch '2.7':
    bpo-29347: Fix possibly dereferencing undefined pointers when creating weakref objects (#128) (#187)
    7131a73

    @zhangyangyu
    Copy link
    Member

    New changeset 9a4577a by GitHub in branch '3.6':
    bpo-29347: Fix possibly dereferencing undefined pointers when creating weakref objects (#128) (#186)
    9a4577a

    @zhangyangyu
    Copy link
    Member

    New changeset 7c95a94 by GitHub in branch '3.5':
    bpo-29347: Fix possibly dereferencing undefined pointers when creating weakref objects (#128) (#188)
    7c95a94

    @zhangyangyu
    Copy link
    Member

    Although no feedback from Saida, but IMHO the problem is solved so I close it now.

    @dhanavaths
    Copy link
    Mannequin Author

    dhanavaths mannequin commented Feb 21, 2017

    Hi Xiang,

    Sorry for the delay. I have not checked my inbox since last week. The proposed fix works for me.

    @zhangyangyu
    Copy link
    Member

    Thanks for your confirmation Saida! :-)

    @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.7 (EOL) end of life interpreter-core (Objects, Python, Grammar, and Parser dirs) type-crash A hard crash of the interpreter, possibly with a core dump
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants