This is an automated email from the git hooks/post-receive script.

git pushed a commit to branch master
in repository enlightenment.

View the commit online.

commit 3a346a09458b01c6c609bd4b2c27e18e636b75fd
Author: [email protected] <[email protected]>
AuthorDate: Sun Mar 15 15:39:51 2026 -0600

    fix: plug use-after-free and dangling signal handlers in NetworkManager
    
    Three memory-safety bugs identified via code review:
    
    1. NM_Access_Point had no Eldbus_Pending* for its initial GetAll call.
       If an AP was removed (AccessPointRemoved signal) while the D-Bus reply
       was still in-flight, _ap_free() would proceed without cancelling the
       pending call, leaving a raw pointer to freed memory as the callback's
       data argument. Added pending_get_props field, cancel it in _ap_free(),
       and suppress the spurious WRN log for the ELDBUS_ERROR_PENDING_CANCELED
       path that results from the synchronous cancel dispatch.
    
    2. _manager_new() registered three signal handlers (PropertiesChanged,
       DeviceAdded, DeviceRemoved) whose return values were discarded, making
       them impossible to unregister. On NetworkManager restart the old nm
       struct is freed while those handlers still hold a raw pointer to it;
       any signal arriving before Eldbus tears down the connection would
       cause a use-after-free. Store all three in NM_Manager and delete them
       in _manager_free() before unreffing the proxies.
    
    3. _manager_free() called eina_stringshare_del on active_ap_path and
       active_connection_path after _manager_active_conn_watch_free() had
       already freed and NULLed both. Remove the redundant calls and add a
       comment explaining the invariant.
    
    Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
---
 .gitignore                                    |  2 ++
 src/modules/networkmanager/e_networkmanager.c | 51 +++++++++++++++++++++------
 src/modules/networkmanager/e_networkmanager.h |  5 +++
 3 files changed, 47 insertions(+), 11 deletions(-)

diff --git a/.gitignore b/.gitignore
index 5a0024426..870db6e53 100644
--- a/.gitignore
+++ b/.gitignore
@@ -10,4 +10,6 @@
 Session.vim
 /build
 .cache
+.claude
+.superpowers
 *~
diff --git a/src/modules/networkmanager/e_networkmanager.c b/src/modules/networkmanager/e_networkmanager.c
index a9e84cfa3..405311e75 100644
--- a/src/modules/networkmanager/e_networkmanager.c
+++ b/src/modules/networkmanager/e_networkmanager.c
@@ -95,6 +95,12 @@ _ap_free(struct NM_Access_Point *ap)
 
    if (!ap) return;
 
+   if (ap->pending_get_props)
+     {
+        eldbus_pending_cancel(ap->pending_get_props);
+        ap->pending_get_props = NULL;
+     }
+
    free(ap->ssid);
    eina_stringshare_del(ap->path);
 
@@ -121,9 +127,12 @@ _ap_get_props_cb(void *data, const Eldbus_Message *msg,
    Eldbus_Message_Iter *array, *dict;
    const char *name, *text;
 
+   ap->pending_get_props = NULL;
+
    if (eldbus_message_error_get(msg, &name, &text))
      {
-        WRN("Could not get AP properties. %s: %s", name, text);
+        if (strcmp(name, ELDBUS_ERROR_PENDING_CANCELED) != 0)
+          WRN("Could not get AP properties. %s: %s", name, text);
         return;
      }
 
@@ -253,8 +262,9 @@ _ap_new(const char *path)
                                       _ap_prop_changed, ap);
 
    /* Fetch all AP properties */
-   eldbus_proxy_call(ap->proxy, "GetAll", _ap_get_props_cb, ap, -1,
-                     "s", NM_IFACE_AP);
+   ap->pending_get_props =
+      eldbus_proxy_call(ap->proxy, "GetAll", _ap_get_props_cb, ap, -1,
+                        "s", NM_IFACE_AP);
 
    return ap;
 }
@@ -1655,14 +1665,17 @@ _manager_new(void)
    nm->props_proxy = eldbus_proxy_get(obj, NM_IFACE_PROPS);
 
    /* PropertiesChanged on the org.freedesktop.DBus.Properties iface */
-   eldbus_proxy_signal_handler_add(nm->props_proxy, "PropertiesChanged",
-                                   _manager_prop_changed, nm);
+   nm->prop_changed_handler =
+      eldbus_proxy_signal_handler_add(nm->props_proxy, "PropertiesChanged",
+                                      _manager_prop_changed, nm);
 
    /* DeviceAdded / DeviceRemoved on NM iface */
-   eldbus_proxy_signal_handler_add(nm->proxy, "DeviceAdded",
-                                   _manager_device_added, nm);
-   eldbus_proxy_signal_handler_add(nm->proxy, "DeviceRemoved",
-                                   _manager_device_removed, nm);
+   nm->device_added_handler =
+      eldbus_proxy_signal_handler_add(nm->proxy, "DeviceAdded",
+                                      _manager_device_added, nm);
+   nm->device_removed_handler =
+      eldbus_proxy_signal_handler_add(nm->proxy, "DeviceRemoved",
+                                      _manager_device_removed, nm);
 
    nm->pending.get_props =
       eldbus_proxy_call(nm->props_proxy, "GetAll",
@@ -1724,8 +1737,8 @@ _manager_free(struct NM_Manager *nm)
      }
 
    free(nm->ip_address);
-   eina_stringshare_del(nm->active_ap_path);
-   eina_stringshare_del(nm->active_connection_path);
+   /* active_ap_path and active_connection_path already freed+NULLed
+    * by _manager_active_conn_watch_free() above */
 
    /* Tear down the Settings signal subscription */
    if (nm->conn_removed_handler)
@@ -1749,6 +1762,22 @@ _manager_free(struct NM_Manager *nm)
         nm->settings_obj = NULL;
      }
 
+   if (nm->prop_changed_handler)
+     {
+        eldbus_signal_handler_del(nm->prop_changed_handler);
+        nm->prop_changed_handler = NULL;
+     }
+   if (nm->device_added_handler)
+     {
+        eldbus_signal_handler_del(nm->device_added_handler);
+        nm->device_added_handler = NULL;
+     }
+   if (nm->device_removed_handler)
+     {
+        eldbus_signal_handler_del(nm->device_removed_handler);
+        nm->device_removed_handler = NULL;
+     }
+
    obj = eldbus_proxy_object_get(nm->proxy);
    eldbus_proxy_unref(nm->proxy);
    eldbus_proxy_unref(nm->props_proxy);
diff --git a/src/modules/networkmanager/e_networkmanager.h b/src/modules/networkmanager/e_networkmanager.h
index e18b4a221..c08ad3f08 100644
--- a/src/modules/networkmanager/e_networkmanager.h
+++ b/src/modules/networkmanager/e_networkmanager.h
@@ -59,6 +59,7 @@ struct NM_Access_Point
    EINA_INLIST;
 
    Eldbus_Signal_Handler *prop_changed_handler;
+   Eldbus_Pending        *pending_get_props;
 
    char         *ssid;
    uint8_t       strength;
@@ -102,6 +103,10 @@ struct NM_Manager
    Eldbus_Proxy *proxy;       /* org.freedesktop.NetworkManager */
    Eldbus_Proxy *props_proxy; /* org.freedesktop.DBus.Properties on NM */
 
+   Eldbus_Signal_Handler *prop_changed_handler;   /* PropertiesChanged on props_proxy */
+   Eldbus_Signal_Handler *device_added_handler;   /* DeviceAdded on proxy */
+   Eldbus_Signal_Handler *device_removed_handler; /* DeviceRemoved on proxy */
+
    Eina_Inlist  *devices;    /* NM_Device inlist */
 
    enum NM_State state;

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.

Reply via email to