On Thu, Feb 16, 2023 at 03:09:02PM +0100, Laszlo Ersek wrote: > 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?
Yes I think it would leak. Sent the fix as another patch. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v _______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://listman.redhat.com/mailman/listinfo/libguestfs
