[Python-checkins] [3.6] bpo-31728: Prevent crashes in _elementtree due to unsafe cleanup of Element.text and Element.tail (GH-3924) (#3945)
Serhiy Storchaka
webhook-mailer at python.org
Tue Oct 10 17:51:31 EDT 2017
https://github.com/python/cpython/commit/a8ac71d15f2a4aef83a8954a678cc12545ce517c
commit: a8ac71d15f2a4aef83a8954a678cc12545ce517c
branch: 3.6
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: Serhiy Storchaka <storchaka at gmail.com>
date: 2017-10-11T00:51:28+03:00
summary:
[3.6] bpo-31728: Prevent crashes in _elementtree due to unsafe cleanup of Element.text and Element.tail (GH-3924) (#3945)
(cherry picked from commit 39ecb9c71b6e55d8a61a710d0144231bd88f9ada)
files:
A Misc/NEWS.d/next/Library/2017-10-08-23-28-30.bpo-31728.XrVMME.rst
M Lib/test/test_xml_etree_c.py
M Modules/_elementtree.c
diff --git a/Lib/test/test_xml_etree_c.py b/Lib/test/test_xml_etree_c.py
index b2200d31f59..d99b4e7527f 100644
--- a/Lib/test/test_xml_etree_c.py
+++ b/Lib/test/test_xml_etree_c.py
@@ -84,6 +84,38 @@ def parser_ref_cycle():
# and so destroy the parser
support.gc_collect()
+ def test_bpo_31728(self):
+ # A crash or an assertion failure shouldn't happen, in case garbage
+ # collection triggers a call to clear() or a reading of text or tail,
+ # while a setter or clear() or __setstate__() is already running.
+ elem = cET.Element('elem')
+ class X:
+ def __del__(self):
+ elem.text
+ elem.tail
+ elem.clear()
+
+ elem.text = X()
+ elem.clear() # shouldn't crash
+
+ elem.tail = X()
+ elem.clear() # shouldn't crash
+
+ elem.text = X()
+ elem.text = X() # shouldn't crash
+ elem.clear()
+
+ elem.tail = X()
+ elem.tail = X() # shouldn't crash
+ elem.clear()
+
+ elem.text = X()
+ elem.__setstate__({'tag': 42}) # shouldn't cause an assertion failure
+ elem.clear()
+
+ elem.tail = X()
+ elem.__setstate__({'tag': 42}) # shouldn't cause an assertion failure
+
@unittest.skipUnless(cET, 'requires _elementtree')
class TestAliasWorking(unittest.TestCase):
diff --git a/Misc/NEWS.d/next/Library/2017-10-08-23-28-30.bpo-31728.XrVMME.rst b/Misc/NEWS.d/next/Library/2017-10-08-23-28-30.bpo-31728.XrVMME.rst
new file mode 100644
index 00000000000..b317d9f210b
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2017-10-08-23-28-30.bpo-31728.XrVMME.rst
@@ -0,0 +1,2 @@
+Prevent crashes in `_elementtree` due to unsafe cleanup of `Element.text`
+and `Element.tail`. Patch by Oren Milman.
diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c
index a6d6c61dd24..707ab2912b3 100644
--- a/Modules/_elementtree.c
+++ b/Modules/_elementtree.c
@@ -61,15 +61,22 @@ do { memory -= size; printf("%8d - %s\n", memory, comment); } while (0)
#define JOIN_SET(p, flag) ((void*) ((uintptr_t) (JOIN_OBJ(p)) | (flag)))
#define JOIN_OBJ(p) ((PyObject*) ((uintptr_t) (p) & ~(uintptr_t)1))
+/* Py_SETREF for a PyObject* that uses a join flag. */
+Py_LOCAL_INLINE(void)
+_set_joined_ptr(PyObject **p, PyObject *new_joined_ptr)
+{
+ PyObject *tmp = JOIN_OBJ(*p);
+ *p = new_joined_ptr;
+ Py_DECREF(tmp);
+}
+
/* Py_CLEAR for a PyObject* that uses a join flag. Pass the pointer by
* reference since this function sets it to NULL.
*/
static void _clear_joined_ptr(PyObject **p)
{
if (*p) {
- PyObject *tmp = JOIN_OBJ(*p);
- *p = NULL;
- Py_DECREF(tmp);
+ _set_joined_ptr(p, NULL);
}
}
@@ -356,7 +363,6 @@ static int
element_init(PyObject *self, PyObject *args, PyObject *kwds)
{
PyObject *tag;
- PyObject *tmp;
PyObject *attrib = NULL;
ElementObject *self_elem;
@@ -397,15 +403,11 @@ element_init(PyObject *self, PyObject *args, PyObject *kwds)
Py_INCREF(tag);
Py_XSETREF(self_elem->tag, tag);
- tmp = self_elem->text;
Py_INCREF(Py_None);
- self_elem->text = Py_None;
- Py_DECREF(JOIN_OBJ(tmp));
+ _set_joined_ptr(&self_elem->text, Py_None);
- tmp = self_elem->tail;
Py_INCREF(Py_None);
- self_elem->tail = Py_None;
- Py_DECREF(JOIN_OBJ(tmp));
+ _set_joined_ptr(&self_elem->tail, Py_None);
return 0;
}
@@ -675,12 +677,10 @@ _elementtree_Element_clear_impl(ElementObject *self)
dealloc_extra(self);
Py_INCREF(Py_None);
- Py_DECREF(JOIN_OBJ(self->text));
- self->text = Py_None;
+ _set_joined_ptr(&self->text, Py_None);
Py_INCREF(Py_None);
- Py_DECREF(JOIN_OBJ(self->tail));
- self->tail = Py_None;
+ _set_joined_ptr(&self->tail, Py_None);
Py_RETURN_NONE;
}
@@ -702,13 +702,11 @@ _elementtree_Element___copy___impl(ElementObject *self)
if (!element)
return NULL;
- Py_DECREF(JOIN_OBJ(element->text));
- element->text = self->text;
- Py_INCREF(JOIN_OBJ(element->text));
+ Py_INCREF(JOIN_OBJ(self->text));
+ _set_joined_ptr(&element->text, self->text);
- Py_DECREF(JOIN_OBJ(element->tail));
- element->tail = self->tail;
- Py_INCREF(JOIN_OBJ(element->tail));
+ Py_INCREF(JOIN_OBJ(self->tail));
+ _set_joined_ptr(&element->tail, self->tail);
if (self->extra) {
if (element_resize(element, self->extra->length) < 0) {
@@ -776,14 +774,12 @@ _elementtree_Element___deepcopy__(ElementObject *self, PyObject *memo)
text = deepcopy(JOIN_OBJ(self->text), memo);
if (!text)
goto error;
- Py_DECREF(element->text);
- element->text = JOIN_SET(text, JOIN_GET(self->text));
+ _set_joined_ptr(&element->text, JOIN_SET(text, JOIN_GET(self->text)));
tail = deepcopy(JOIN_OBJ(self->tail), memo);
if (!tail)
goto error;
- Py_DECREF(element->tail);
- element->tail = JOIN_SET(tail, JOIN_GET(self->tail));
+ _set_joined_ptr(&element->tail, JOIN_SET(tail, JOIN_GET(self->tail)));
if (self->extra) {
if (element_resize(element, self->extra->length) < 0)
@@ -967,13 +963,13 @@ element_setstate_from_attributes(ElementObject *self,
Py_INCREF(tag);
Py_XSETREF(self->tag, tag);
- _clear_joined_ptr(&self->text);
- self->text = text ? JOIN_SET(text, PyList_CheckExact(text)) : Py_None;
- Py_INCREF(JOIN_OBJ(self->text));
+ text = text ? JOIN_SET(text, PyList_CheckExact(text)) : Py_None;
+ Py_INCREF(JOIN_OBJ(text));
+ _set_joined_ptr(&self->text, text);
- _clear_joined_ptr(&self->tail);
- self->tail = tail ? JOIN_SET(tail, PyList_CheckExact(tail)) : Py_None;
- Py_INCREF(JOIN_OBJ(self->tail));
+ tail = tail ? JOIN_SET(tail, PyList_CheckExact(tail)) : Py_None;
+ Py_INCREF(JOIN_OBJ(tail));
+ _set_joined_ptr(&self->tail, tail);
/* Handle ATTRIB and CHILDREN. */
if (!children && !attrib)
@@ -1980,8 +1976,7 @@ element_text_setter(ElementObject *self, PyObject *value, void *closure)
{
_VALIDATE_ATTR_VALUE(value);
Py_INCREF(value);
- Py_DECREF(JOIN_OBJ(self->text));
- self->text = value;
+ _set_joined_ptr(&self->text, value);
return 0;
}
@@ -1990,8 +1985,7 @@ element_tail_setter(ElementObject *self, PyObject *value, void *closure)
{
_VALIDATE_ATTR_VALUE(value);
Py_INCREF(value);
- Py_DECREF(JOIN_OBJ(self->tail));
- self->tail = value;
+ _set_joined_ptr(&self->tail, value);
return 0;
}
More information about the Python-checkins
mailing list