Re: [Libguestfs] [PATCH 1/2] python: Avoid crash if callback parameters cannot be built

On 2/14/23 19:51, Richard W.M. Jones wrote: > In the case that building the parameters to the Python event callback > fails, args was returned as NULL. We immediately tried to call > Py_INCREF on this which crashed. Returning NULL means the Python > function threw an exception, so print the exception and return (there > is no way to return an error here - the event is lost). > > Reported-by: Yonatan Shtarkman > See: https://listman.redhat.com/archives/libguestfs/2023-February/030653.html > --- > python/handle.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/python/handle.c b/python/handle.c > index c8752baf47..f37e939e03 100644 > --- a/python/handle.c > +++ b/python/handle.c > @@ -134,18 +134,21 @@ guestfs_int_py_event_callback_wrapper (guestfs_h *g, > args = Py_BuildValue ("(Kis#O)", > (unsigned PY_LONG_LONG) event, event_handle, > buf, buf_len, py_array); > + if (args == NULL) { > + PyErr_PrintEx (0); > + goto out; > + } > + > Py_INCREF (args); > - > py_r = PyObject_CallObject (py_callback, args); > - > Py_DECREF (args); > - > if (py_r != NULL) > Py_DECREF (py_r); > else > /* Callback threw an exception: print it. */ > PyErr_PrintEx (0); > > + out: > PyGILState_Release (py_save); > } > My understanding of object references in this function is the following: - PyList_New creates a new object / new reference "py_array" - Py_BuildValue with the "O" format specifier transfers the new list's *sole* reference (= ownership) to the just-built higher-level object "args" - when "args" is killed (decref'd), it takes care of "py_array". Consequently, if Py_BuildValue fails, "py_array" continues owning the new list -- and I believe that, if we take the new error branch, we leak the object pointed-to by "py_array". Is that the case?