classification
Title: Python could crash while creating weakref for a given object
Type: crash Stage: resolved
Components: Interpreter Core Versions: Python 3.7, Python 3.6, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: christian.heimes, dhanavaths, fdrake, xiang.zhang
Priority: normal Keywords: patch

Created on 2017-01-23 03:24 by dhanavaths, last changed 2017-02-21 05:54 by xiang.zhang. This issue is now closed.

Files
File name Uploaded Description Edit
crash_in_weakref.zip dhanavaths, 2017-01-23 21:06
weakref_crash.patch xiang.zhang, 2017-02-04 08:02 review
verify_crash_in_weakref.zip dhanavaths, 2017-02-04 10:39
weakref_crash.c xiang.zhang, 2017-02-04 14:16
Pull Requests
URL Status Linked Edit
PR 128 merged xiang.zhang, 2017-02-16 04:44
PR 186 merged xiang.zhang, 2017-02-20 04:58
PR 187 merged xiang.zhang, 2017-02-20 04:58
PR 188 merged xiang.zhang, 2017-02-20 04:59
Messages (19)
msg286044 - (view) Author: Saida Dhanavath (dhanavaths) Date: 2017-01-23 03:23
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.
msg286062 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2017-01-23 08:56
Can you reproduce the issue with a more recent version of Python 2.7? 2.7.8 is pretty old.
msg286116 - (view) Author: Saida Dhanavath (dhanavaths) Date: 2017-01-23 21:06
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.
msg286133 - (view) Author: Saida Dhanavath (dhanavaths) Date: 2017-01-24 04:50
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)
msg286926 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-04 08:02
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
msg286935 - (view) Author: Saida Dhanavath (dhanavaths) Date: 2017-02-04 09:18
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.
msg286948 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-04 10:08
Hmm, what's your test program? Would you mind show it?
msg286953 - (view) Author: Saida Dhanavath (dhanavaths) Date: 2017-02-04 10:39
Please find the test program attached. Readme.txt has steps to comiple and run program.
msg286956 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-04 11:02
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.
msg286964 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-04 14:16
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. :-)
msg286965 - (view) Author: Saida Dhanavath (dhanavaths) Date: 2017-02-04 14:30
Xiang, 


Sure, I will run it with other python versions and post the results.
msg287912 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-16 04:25
Ping.
msg288180 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-20 04:25
New changeset d0e8212ed70445cc3d48b0d4ae7c9cb480004010 by GitHub in branch 'master':
bpo-29347: Fix possibly dereferencing undefined pointers when creating weakref objects (#128)
https://github.com/python/cpython/commit/d0e8212ed70445cc3d48b0d4ae7c9cb480004010
msg288181 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-20 06:32
New changeset 7131a73f9655cfd325c798385905326f57b94640 by GitHub in branch '2.7':
bpo-29347: Fix possibly dereferencing undefined pointers when creating weakref objects (#128) (#187)
https://github.com/python/cpython/commit/7131a73f9655cfd325c798385905326f57b94640
msg288182 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-20 06:33
New changeset 9a4577a4bb23888fed2cf192cf1a4c95ce5c26f8 by GitHub in branch '3.6':
bpo-29347: Fix possibly dereferencing undefined pointers when creating weakref objects (#128) (#186)
https://github.com/python/cpython/commit/9a4577a4bb23888fed2cf192cf1a4c95ce5c26f8
msg288183 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-20 06:33
New changeset 7c95a94c3ab41e4296e94335d66b2400ad16f052 by GitHub in branch '3.5':
bpo-29347: Fix possibly dereferencing undefined pointers when creating weakref objects (#128) (#188)
https://github.com/python/cpython/commit/7c95a94c3ab41e4296e94335d66b2400ad16f052
msg288184 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-20 06:35
Although no feedback from Saida, but IMHO the problem is solved so I close it now.
msg288270 - (view) Author: Saida Dhanavath (dhanavaths) Date: 2017-02-21 05:53
Hi Xiang,


Sorry for the delay. I have not checked my inbox since last week. The proposed fix works for me.
msg288271 - (view) Author: Xiang Zhang (xiang.zhang) * (Python committer) Date: 2017-02-21 05:54
Thanks for your confirmation Saida! :-)
History
Date User Action Args
2017-02-21 05:54:59xiang.zhangsetmessages: + msg288271
2017-02-21 05:53:55dhanavathssetmessages: + msg288270
2017-02-20 06:35:53xiang.zhangsetstatus: open -> closed
resolution: fixed
messages: + msg288184

stage: patch review -> resolved
2017-02-20 06:33:08xiang.zhangsetmessages: + msg288183
2017-02-20 06:33:04xiang.zhangsetmessages: + msg288182
2017-02-20 06:32:55xiang.zhangsetmessages: + msg288181
2017-02-20 04:59:30xiang.zhangsetpull_requests: + pull_request154
2017-02-20 04:58:51xiang.zhangsetpull_requests: + pull_request153
2017-02-20 04:58:00xiang.zhangsetpull_requests: + pull_request152
2017-02-20 04:25:18xiang.zhangsetmessages: + msg288180
2017-02-16 04:44:30xiang.zhangsetpull_requests: + pull_request90
2017-02-16 04:25:41xiang.zhangsetmessages: + msg287912
2017-02-04 14:30:42dhanavathssetmessages: + msg286965
2017-02-04 14:16:16xiang.zhangsetfiles: + weakref_crash.c

messages: + msg286964
2017-02-04 11:02:42xiang.zhangsetmessages: + msg286956
2017-02-04 10:39:57dhanavathssetfiles: + verify_crash_in_weakref.zip

messages: + msg286953
2017-02-04 10:08:36xiang.zhangsetmessages: + msg286948
2017-02-04 09:18:40dhanavathssetmessages: + msg286935
2017-02-04 08:05:20xiang.zhangsetnosy: + fdrake
2017-02-04 08:02:37xiang.zhangsetfiles: + weakref_crash.patch
title: Python 2.7.8 is crashing while creating weakref for a given object. -> Python could crash while creating weakref for a given object
messages: + msg286926

keywords: + patch
stage: patch review
2017-02-03 15:14:25xiang.zhangsetnosy: + xiang.zhang

versions: - Python 3.3, Python 3.4
2017-02-02 18:00:42dhanavathssetversions: + Python 3.3, Python 3.4, Python 3.5, Python 3.6, Python 3.7
2017-01-24 04:50:14dhanavathssetmessages: + msg286133
2017-01-23 21:06:16dhanavathssetfiles: + crash_in_weakref.zip

messages: + msg286116
2017-01-23 08:56:54christian.heimessetnosy: + christian.heimes
messages: + msg286062
2017-01-23 03:32:13dhanavathssettype: crash
2017-01-23 03:24:01dhanavathscreate