Skip to content

Commit

Permalink
gh-111784: Fix two segfaults in the elementtree module (GH-113405)
Browse files Browse the repository at this point in the history
First fix resolve situation when pyexpat module (which contains expat_CAPI
capsule) deallocates before _elementtree, so we need to hold a strong
reference to pyexpat module to.

Second fix resolve situation when module state is deallocated before
deallocation of XMLParser instances, which uses module state to clear
some stuff.
  • Loading branch information
Eclips4 committed Dec 24, 2023
1 parent 050783c commit 894f0e5
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fix segfaults in the ``_elementtree`` module.
Fix first segfault during deallocation of ``_elementtree.XMLParser`` instances by keeping strong reference
to ``pyexpat`` module in module state for capsule lifetime.
Fix second segfault which happens in the same deallocation process by keeping strong reference
to ``_elementtree`` module in ``XMLParser`` structure for ``_elementtree`` module lifetime.
16 changes: 14 additions & 2 deletions Modules/_elementtree.c
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ typedef struct {
PyTypeObject *TreeBuilder_Type;
PyTypeObject *XMLParser_Type;

PyObject *expat_capsule;
struct PyExpat_CAPI *expat_capi;
} elementtreestate;

Expand Down Expand Up @@ -155,6 +156,7 @@ elementtree_clear(PyObject *m)
Py_CLEAR(st->ElementIter_Type);
Py_CLEAR(st->TreeBuilder_Type);
Py_CLEAR(st->XMLParser_Type);
Py_CLEAR(st->expat_capsule);

st->expat_capi = NULL;
return 0;
Expand All @@ -175,6 +177,7 @@ elementtree_traverse(PyObject *m, visitproc visit, void *arg)
Py_VISIT(st->ElementIter_Type);
Py_VISIT(st->TreeBuilder_Type);
Py_VISIT(st->XMLParser_Type);
Py_VISIT(st->expat_capsule);
return 0;
}

Expand Down Expand Up @@ -3066,6 +3069,7 @@ typedef struct {
PyObject *handle_close;

elementtreestate *state;
PyObject *elementtree_module;
} XMLParserObject;

/* helpers */
Expand Down Expand Up @@ -3607,7 +3611,11 @@ xmlparser_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
self->handle_start = self->handle_data = self->handle_end = NULL;
self->handle_comment = self->handle_pi = self->handle_close = NULL;
self->handle_doctype = NULL;
self->state = get_elementtree_state_by_type(type);
self->elementtree_module = PyType_GetModuleByDef(type, &elementtreemodule);
assert(self->elementtree_module != NULL);
Py_INCREF(self->elementtree_module);
// See gh-111784 for explanation why is reference to module needed here.
self->state = get_elementtree_state(self->elementtree_module);
}
return (PyObject *)self;
}
Expand Down Expand Up @@ -3784,6 +3792,7 @@ xmlparser_gc_clear(XMLParserObject *self)
EXPAT(st, ParserFree)(parser);
}

Py_CLEAR(self->elementtree_module);
Py_CLEAR(self->handle_close);
Py_CLEAR(self->handle_pi);
Py_CLEAR(self->handle_comment);
Expand Down Expand Up @@ -4343,7 +4352,10 @@ module_exec(PyObject *m)
goto error;

/* link against pyexpat */
st->expat_capi = PyCapsule_Import(PyExpat_CAPSULE_NAME, 0);
if (!(st->expat_capsule = _PyImport_GetModuleAttrString("pyexpat", "expat_CAPI")))
goto error;
if (!(st->expat_capi = PyCapsule_GetPointer(st->expat_capsule, PyExpat_CAPSULE_NAME)))
goto error;
if (st->expat_capi) {
/* check that it's usable */
if (strcmp(st->expat_capi->magic, PyExpat_CAPI_MAGIC) != 0 ||
Expand Down

0 comments on commit 894f0e5

Please sign in to comment.