shell/camera-monitor: Remove hooks before destroying objects (!2893) · Merge requests · GNOME / gnome-shell

Sebastian Keller requested to merge skeller/gnome-shell:fix-camera-monitor-invalid-access into main Aug 15, 2023

Otherwise removing the hook will try to unlink from a list whose
previous element belonged to the already destroyed object.

Fixes: d09d24666 ("shell: Add device monitor for cameras")

From valgrind:

==5813== Invalid write of size 8
==5813==    at 0x4879E61: spa_list_remove (list.h:61)
==5813==    by 0x4879E61: spa_hook_remove (hook.h:366)
==5813==    by 0x4879E61: shell_camera_monitor_disconnect_core (shell-camera-monitor.c:303)
==5813==    by 0x4879F30: shell_camera_monitor_finalize (shell-camera-monitor.c:377)
==5813==    by 0x4E2567E: g_object_unref (gobject.c:3941)
==5813==    by 0x4E2567E: g_object_unref (gobject.c:3805)
==5813==    by 0x4F00CA2: reset (jsapi-util.h:256)
==5813==    by 0x4F00CA2: ~GjsAutoPointer (jsapi-util.h:187)
==5813==    by 0x4F00CA2: ~GjsSmartPointer (jsapi-util.h:382)
==5813==    by 0x4F00CA2: release_native_object (object.cpp:1526)
==5813==    by 0x4F00CA2: ObjectInstance::release_native_object() (object.cpp:1502)
==5813==    by 0x4F0328D: ObjectInstance::disassociate_js_gobject() (object.cpp:1752)
==5813==    by 0x4F014AA: operator() (std_function.h:591)
==5813==    by 0x4F014AA: operator() (object.cpp:1307)
==5813==    by 0x4F014AA: operator()<__gnu_cxx::__normal_iterator<ObjectInstance**, std::vector<ObjectInstance*> > > (predefined_ops.h:318)
==5813==    by 0x4F014AA: __remove_if<__gnu_cxx::__normal_iterator<ObjectInstance**, std::vector<ObjectInstance*> >, __gnu_cxx::__ops::_Iter_pred<ObjectInstance::remove_wrapped_gobjects_if(const Predicate&, const Action&)::<lambda(ObjectInstance*)> > > (stl_algobase.h:2145)
==5813==    by 0x4F014AA: remove_if<__gnu_cxx::__normal_iterator<ObjectInstance**, std::vector<ObjectInstance*> >, ObjectInstance::remove_wrapped_gobjects_if(const Predicate&, const Action&)::<lambda(ObjectInstance*)> > (stl_algo.h:880)
==5813==    by 0x4F014AA: ObjectInstance::remove_wrapped_gobjects_if(std::function<bool (ObjectInstance*)> const&, std::function<void (ObjectInstance*)> const&) (object.cpp:1303)
==5813==    by 0x4F03570: ObjectInstance::update_heap_wrapper_weak_pointers(JSTracer*, JS::Compartment*, void*) (object.cpp:1600)
==5813==    by 0x65FA30C: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==    by 0x65EBBBB: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==    by 0x6602C6C: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==    by 0x65C8F29: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==    by 0x65CCA4F: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==  Address 0x30ad2908 is 88 bytes inside a block of size 384 free'd
==5813==    at 0x48452AC: free (vg_replace_malloc.c:974)
==5813==    by 0x5C1402C: pw_core_disconnect (core.c:473)
==5813==    by 0x4879E53: shell_camera_monitor_disconnect_core (shell-camera-monitor.c:302)
==5813==    by 0x4879F30: shell_camera_monitor_finalize (shell-camera-monitor.c:377)
==5813==    by 0x4E2567E: g_object_unref (gobject.c:3941)
==5813==    by 0x4E2567E: g_object_unref (gobject.c:3805)
==5813==    by 0x4F00CA2: reset (jsapi-util.h:256)
==5813==    by 0x4F00CA2: ~GjsAutoPointer (jsapi-util.h:187)
==5813==    by 0x4F00CA2: ~GjsSmartPointer (jsapi-util.h:382)
==5813==    by 0x4F00CA2: release_native_object (object.cpp:1526)
==5813==    by 0x4F00CA2: ObjectInstance::release_native_object() (object.cpp:1502)
==5813==    by 0x4F0328D: ObjectInstance::disassociate_js_gobject() (object.cpp:1752)
==5813==    by 0x4F014AA: operator() (std_function.h:591)
==5813==    by 0x4F014AA: operator() (object.cpp:1307)
==5813==    by 0x4F014AA: operator()<__gnu_cxx::__normal_iterator<ObjectInstance**, std::vector<ObjectInstance*> > > (predefined_ops.h:318)
==5813==    by 0x4F014AA: __remove_if<__gnu_cxx::__normal_iterator<ObjectInstance**, std::vector<ObjectInstance*> >, __gnu_cxx::__ops::_Iter_pred<ObjectInstance::remove_wrapped_gobjects_if(const Predicate&, const Action&)::<lambda(ObjectInstance*)> > > (stl_algobase.h:2145)
==5813==    by 0x4F014AA: remove_if<__gnu_cxx::__normal_iterator<ObjectInstance**, std::vector<ObjectInstance*> >, ObjectInstance::remove_wrapped_gobjects_if(const Predicate&, const Action&)::<lambda(ObjectInstance*)> > (stl_algo.h:880)
==5813==    by 0x4F014AA: ObjectInstance::remove_wrapped_gobjects_if(std::function<bool (ObjectInstance*)> const&, std::function<void (ObjectInstance*)> const&) (object.cpp:1303)
==5813==    by 0x4F03570: ObjectInstance::update_heap_wrapper_weak_pointers(JSTracer*, JS::Compartment*, void*) (object.cpp:1600)
==5813==    by 0x65FA30C: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==    by 0x65EBBBB: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==    by 0x6602C6C: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==  Block was alloc'd at
==5813==    at 0x484782C: calloc (vg_replace_malloc.c:1554)
==5813==    by 0x5C130CA: core_new (core.c:296)
==5813==    by 0x5C1407B: pw_context_connect (core.c:388)
==5813==    by 0x4879978: shell_camera_monitor_connect_core (shell-camera-monitor.c:275)
==5813==    by 0x4879B0E: shell_camera_monitor_init (shell-camera-monitor.c:448)
==5813==    by 0x4E422F0: g_type_create_instance (gtype.c:1997)
==5813==    by 0x4E26033: g_object_new_internal.part.0 (gobject.c:2245)
==5813==    by 0x4E27512: g_object_new_internal (gobject.c:2242)
==5813==    by 0x4E27512: g_object_new_with_properties (gobject.c:2408)
==5813==    by 0x4F0C278: ObjectInstance::init_impl(JSContext*, JS::CallArgs const&, JS::Handle<JSObject*>) (object.cpp:1803)
==5813==    by 0x4F0C7E8: ObjectBase::init_gobject(JSContext*, unsigned int, JS::Value*) (object.cpp:2531)
==5813==    by 0x61D96C8: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==    by 0x61D9A4C: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)

==5813== Invalid write of size 8
==5813==    at 0x4879E68: spa_list_remove (list.h:62)
==5813==    by 0x4879E68: spa_hook_remove (hook.h:366)
==5813==    by 0x4879E68: shell_camera_monitor_disconnect_core (shell-camera-monitor.c:303)
==5813==    by 0x4879F30: shell_camera_monitor_finalize (shell-camera-monitor.c:377)
==5813==    by 0x4E2567E: g_object_unref (gobject.c:3941)
==5813==    by 0x4E2567E: g_object_unref (gobject.c:3805)
==5813==    by 0x4F00CA2: reset (jsapi-util.h:256)
==5813==    by 0x4F00CA2: ~GjsAutoPointer (jsapi-util.h:187)
==5813==    by 0x4F00CA2: ~GjsSmartPointer (jsapi-util.h:382)
==5813==    by 0x4F00CA2: release_native_object (object.cpp:1526)
==5813==    by 0x4F00CA2: ObjectInstance::release_native_object() (object.cpp:1502)
==5813==    by 0x4F0328D: ObjectInstance::disassociate_js_gobject() (object.cpp:1752)
==5813==    by 0x4F014AA: operator() (std_function.h:591)
==5813==    by 0x4F014AA: operator() (object.cpp:1307)
==5813==    by 0x4F014AA: operator()<__gnu_cxx::__normal_iterator<ObjectInstance**, std::vector<ObjectInstance*> > > (predefined_ops.h:318)
==5813==    by 0x4F014AA: __remove_if<__gnu_cxx::__normal_iterator<ObjectInstance**, std::vector<ObjectInstance*> >, __gnu_cxx::__ops::_Iter_pred<ObjectInstance::remove_wrapped_gobjects_if(const Predicate&, const Action&)::<lambda(ObjectInstance*)> > > (stl_algobase.h:2145)
==5813==    by 0x4F014AA: remove_if<__gnu_cxx::__normal_iterator<ObjectInstance**, std::vector<ObjectInstance*> >, ObjectInstance::remove_wrapped_gobjects_if(const Predicate&, const Action&)::<lambda(ObjectInstance*)> > (stl_algo.h:880)
==5813==    by 0x4F014AA: ObjectInstance::remove_wrapped_gobjects_if(std::function<bool (ObjectInstance*)> const&, std::function<void (ObjectInstance*)> const&) (object.cpp:1303)
==5813==    by 0x4F03570: ObjectInstance::update_heap_wrapper_weak_pointers(JSTracer*, JS::Compartment*, void*) (object.cpp:1600)
==5813==    by 0x65FA30C: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==    by 0x65EBBBB: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==    by 0x6602C6C: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==    by 0x65C8F29: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==    by 0x65CCA4F: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==  Address 0x30ad2910 is 96 bytes inside a block of size 384 free'd
==5813==    at 0x48452AC: free (vg_replace_malloc.c:974)
==5813==    by 0x5C1402C: pw_core_disconnect (core.c:473)
==5813==    by 0x4879E53: shell_camera_monitor_disconnect_core (shell-camera-monitor.c:302)
==5813==    by 0x4879F30: shell_camera_monitor_finalize (shell-camera-monitor.c:377)
==5813==    by 0x4E2567E: g_object_unref (gobject.c:3941)
==5813==    by 0x4E2567E: g_object_unref (gobject.c:3805)
==5813==    by 0x4F00CA2: reset (jsapi-util.h:256)
==5813==    by 0x4F00CA2: ~GjsAutoPointer (jsapi-util.h:187)
==5813==    by 0x4F00CA2: ~GjsSmartPointer (jsapi-util.h:382)
==5813==    by 0x4F00CA2: release_native_object (object.cpp:1526)
==5813==    by 0x4F00CA2: ObjectInstance::release_native_object() (object.cpp:1502)
==5813==    by 0x4F0328D: ObjectInstance::disassociate_js_gobject() (object.cpp:1752)
==5813==    by 0x4F014AA: operator() (std_function.h:591)
==5813==    by 0x4F014AA: operator() (object.cpp:1307)
==5813==    by 0x4F014AA: operator()<__gnu_cxx::__normal_iterator<ObjectInstance**, std::vector<ObjectInstance*> > > (predefined_ops.h:318)
==5813==    by 0x4F014AA: __remove_if<__gnu_cxx::__normal_iterator<ObjectInstance**, std::vector<ObjectInstance*> >, __gnu_cxx::__ops::_Iter_pred<ObjectInstance::remove_wrapped_gobjects_if(const Predicate&, const Action&)::<lambda(ObjectInstance*)> > > (stl_algobase.h:2145)
==5813==    by 0x4F014AA: remove_if<__gnu_cxx::__normal_iterator<ObjectInstance**, std::vector<ObjectInstance*> >, ObjectInstance::remove_wrapped_gobjects_if(const Predicate&, const Action&)::<lambda(ObjectInstance*)> > (stl_algo.h:880)
==5813==    by 0x4F014AA: ObjectInstance::remove_wrapped_gobjects_if(std::function<bool (ObjectInstance*)> const&, std::function<void (ObjectInstance*)> const&) (object.cpp:1303)
==5813==    by 0x4F03570: ObjectInstance::update_heap_wrapper_weak_pointers(JSTracer*, JS::Compartment*, void*) (object.cpp:1600)
==5813==    by 0x65FA30C: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==    by 0x65EBBBB: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==    by 0x6602C6C: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==  Block was alloc'd at
==5813==    at 0x484782C: calloc (vg_replace_malloc.c:1554)
==5813==    by 0x5C130CA: core_new (core.c:296)
==5813==    by 0x5C1407B: pw_context_connect (core.c:388)
==5813==    by 0x4879978: shell_camera_monitor_connect_core (shell-camera-monitor.c:275)
==5813==    by 0x4879B0E: shell_camera_monitor_init (shell-camera-monitor.c:448)
==5813==    by 0x4E422F0: g_type_create_instance (gtype.c:1997)
==5813==    by 0x4E26033: g_object_new_internal.part.0 (gobject.c:2245)
==5813==    by 0x4E27512: g_object_new_internal (gobject.c:2242)
==5813==    by 0x4E27512: g_object_new_with_properties (gobject.c:2408)
==5813==    by 0x4F0C278: ObjectInstance::init_impl(JSContext*, JS::CallArgs const&, JS::Handle<JSObject*>) (object.cpp:1803)
==5813==    by 0x4F0C7E8: ObjectBase::init_gobject(JSContext*, unsigned int, JS::Value*) (object.cpp:2531)
==5813==    by 0x61D96C8: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)
==5813==    by 0x61D9A4C: ??? (in /usr/lib64/libmozjs-115.so.0.0.0)

shell/camera-monitor: Remove hooks before destroying objects

Merge request reports