Issue 16445: SEGFAULT when deleting Exception.message (original) (raw)

Created on 2012-11-08 23:05 by amaury.forgeotdarc, last changed 2022-04-11 14:57 by admin. This issue is now closed.

Messages (13)
msg175203 - (view)	Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) *	Date: 2012-11-08 23:05
The script below segfaults cpython2.7. The cause is in BaseException_set_message(), which calls Py_XDECREF(self->message) and thus can call back into Python code with a dangling PyObject* pointer. Py_CLEAR should be used instead. class Nasty(str): def __del__(self): del e.message e = ValueError(Nasty("msg")) e.args = () del e.message del e
msg175211 - (view)	Author: STINNER Victor (vstinner) *	Date: 2012-11-09 00:59
According to Amaury an IRC, replacing "Py_XDECREF(obj->attr); obj->attr = NULL;" pattern with "Py_CLEAR(obj->attr);" should fix the issue. Instead of opening one issue per occurence of this pattern, it is possible to write a semantic patch to do the replace on the whole CPython project using the Coccinelle tool. http://coccinelle.lip6.fr/ See for example attached py_clear.spatch semantic patch. Install coccinelle and run "spatch -in_place -sp_file py_clear.spatch -dir .". The result is the attached patch (python2.7_pyclear.patch). The patch is huge! Mac/Modules/_scproxy.c | 2 +- Mac/Modules/carbonevt/_CarbonEvtmodule.c	3 +- Mac/Modules/list/_Listmodule.c	3 +- Modules/_bsddb.c	48 ++++--------- Modules/_ctypes/_ctypes.c	18 +--- Modules/_elementtree.c	8 +- Modules/_hotshot.c	3 +- Modules/_localemodule.c	3 +- Modules/_sqlite/connection.c	6 +- Modules/_sqlite/cursor.c	12 +-- Modules/_tkinter.c	6 +- Modules/almodule.c	3 +- Modules/binascii.c	21 ++---- Modules/bz2module.c	12 +-- Modules/cPickle.c	3 +- Modules/cdmodule.c	18 +--- Modules/cjkcodecs/multibytecodec.c	3 +- Modules/datetimemodule.c	12 +-- Modules/flmodule.c	12 +-- Modules/fmmodule.c	3 +- Modules/nismodule.c	3 +- Modules/posixmodule.c	33 +++------ Modules/pyexpat.c	6 +- Modules/readline.c	7 +- Modules/selectmodule.c	3 +- Modules/signalmodule.c	9 +- Modules/svmodule.c	3 +- Modules/syslogmodule.c	3 +- Modules/zlibmodule.c	18 +--- Objects/abstract.c	6 +- Objects/classobject.c	18 +--- Objects/descrobject.c	3 +- Objects/exceptions.c	3 +- Objects/fileobject.c	12 +-- Objects/frameobject.c	3 +- Objects/genobject.c	3 +- Objects/longobject.c	3 +- Objects/object.c	6 +- Objects/stringobject.c	12 +-- Objects/tupleobject.c	6 +- Objects/typeobject.c	12 +-- Objects/unicodeobject.c	12 +-- Python/Python-ast.c	381 ++++++++++++++++++++++++++++++++++++++---------------------------------------------------------------------------- Python/bltinmodule.c	12 +-- Python/errors.c	3 +- Python/import.c	6 +- Python/marshal.c	12 +-- Python/modsupport.c	3 +- Python/structmember.c	3 +- Python/sysmodule.c	9 +- RISCOS/Modules/drawfmodule.c	2 +- RISCOS/Modules/swimodule.c	2 +- 52 files changed, 275 insertions(+), 541 deletions(-)
msg175212 - (view)	Author: STINNER Victor (vstinner) *	Date: 2012-11-09 01:01
The "spatch" doesn't match the following macro: Modules/_testcapimodule.c:#define UNBIND(X) Py_DECREF(X); (X) = NULL -- Python/Python-ast.c is autogenerated from Parser/asdl_c.py, the .py file should be fixed instead.
msg175213 - (view)	Author: STINNER Victor (vstinner) *	Date: 2012-11-09 01:04
python27_pyclear.patch does fix the use case described in , but it doesn't fix #16447.
msg175216 - (view)	Author: STINNER Victor (vstinner) *	Date: 2012-11-09 01:18
py_clear.spatch replaces: Py_DECREF(obj->attr); obj->attr = NULL; but also: Py_DECREF(obj); obj = NULL; If the second pattern is not dangerous, py_clear.spatch can be modified to only match the first pattern: see py_decref_replace.spatch of issue #16447 for an example.
msg175224 - (view)	Author: Serhiy Storchaka (serhiy.storchaka) *	Date: 2012-11-09 11:40
The Coccinelle looks as an amazing tool! I propose split a patch on several patches (autogenerated code, attributes clearing, other pointer expressions (as *exceptionObject or unicode_latin1[i]) clearing, and local pointers clearing at the end) and commit they separately.
msg176910 - (view)	Author: Serhiy Storchaka (serhiy.storchaka) *	Date: 2012-12-04 12:58
Victor, I will be glad to review any related patches, but please split the patch on several patches, from most undoubted to more sophisticated patterns.
msg181839 - (view)	Author: Mark Dickinson (mark.dickinson) *	Date: 2013-02-10 18:52
Can I suggest fixing this particular issue with a dedicated patch, and opening another issue to consider the large automated replacements that Victor's proposing?
msg181840 - (view)	Author: Mark Dickinson (mark.dickinson) *	Date: 2013-02-10 18:56
Here's a simple patch (against 2.7) for this particular issue.
msg181841 - (view)	Author: Serhiy Storchaka (serhiy.storchaka) *	Date: 2013-02-10 19:07
> Here's a simple patch (against 2.7) for this particular issue. LGTM.
msg183368 - (view)	Author: Roundup Robot (python-dev)	Date: 2013-03-03 11:13
New changeset 0e41c4466d58 by Mark Dickinson in branch '2.7': Issue #16445: Fix potential segmentation fault when deleting an exception message. http://hg.python.org/cpython/rev/0e41c4466d58
msg187262 - (view)	Author: Mark Lawrence (BreamoreBoy) *	Date: 2013-04-18 15:51
It looks as if this issue can be closed as a fix has been committed. However a new issue will be needed if the advice offered in is followed.
msg188405 - (view)	Author: Antoine Pitrou (pitrou) *	Date: 2013-05-04 20:26
This looks like it is fixed by Mark's commit. Other proposals should go into separate issues.