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 8d02842ae1ad1d4e0993b51ef6ecfc33bdabcce9
Author: [email protected] <[email protected]>
AuthorDate: Fri Mar 13 15:49:43 2026 -0600
fix: resolve use-after-free and race conditions in NetworkManager forget button
The forget button implementation had multiple concurrent issues that are now
fixed together:
Use-after-free (root cause): When a user clicked the forget button, it called
enm_mod_aps_update_now() to immediately rebuild the UI, which deleted the
forget button widget and triggered EVAS_CALLBACK_DEL. This callback freed the
_Enm_Forget_Data structure including the connection_path stringshare before
enm_connection_delete() could use it. Fixed by saving and ref'ing the path and
nm pointer before the update, then using them after.
Optimistic UI feedback: The forget button now disappears immediately upon click
by removing the SSID from the saved_connections hash and rebuilding popups,
rather than waiting for D-Bus round trips. This provides instant visual
feedback to the user.
Race window on delete: After Connection.Delete() succeeds, NM may not have
immediately removed the connection from ListConnections(), causing subsequent
polls to re-add it to the hash. Fixed by subscribing to the authoritative
org.freedesktop.NetworkManager.Settings.ConnectionRemoved signal, which fires
once NM's internal state is updated.
Failure recovery: If Connection.Delete() fails, we now call
enm_saved_connections_get() to restore the optimistically-removed entry so the
button reappears, informing the user of the failure.
Signal handler data consistency: _settings_conn_removed_cb now correctly uses
the provided data parameter (NM_Manager *) instead of relying on the global
nm_manager variable.
Fixes #unknown (ConnectionRemoved signal handling)
---
src/modules/networkmanager/e_mod_main.c | 48 +++++++++++++++++++++++-
src/modules/networkmanager/e_mod_main.h | 1 +
src/modules/networkmanager/e_networkmanager.c | 54 +++++++++++++++++++++++++--
src/modules/networkmanager/e_networkmanager.h | 6 +++
4 files changed, 104 insertions(+), 5 deletions(-)
diff --git a/src/modules/networkmanager/e_mod_main.c b/src/modules/networkmanager/e_mod_main.c
index 35d1f49f3..46b3ec340 100644
--- a/src/modules/networkmanager/e_mod_main.c
+++ b/src/modules/networkmanager/e_mod_main.c
@@ -182,6 +182,7 @@ struct _Enm_Forget_Data
{
struct NM_Manager *nm;
const char *connection_path; /* stringshare */
+ const char *ssid; /* stringshare — for optimistic hash removal */
};
static void
@@ -190,9 +191,29 @@ _enm_forget_mouse_up_cb(void *data, Evas *e EINA_UNUSED,
void *event_info EINA_UNUSED)
{
struct _Enm_Forget_Data *fd = data;
+ struct NM_Manager *nm = fd->nm;
+ /* Take a ref now: enm_mod_aps_update_now() may delete the forget button
+ * widget (replacing it with NULL), which fires EVAS_CALLBACK_DEL and frees
+ * fd — including its connection_path stringshare — before we use it. */
+ const char *conn_path = eina_stringshare_ref(fd->connection_path);
- INF("Forget connection: %s", fd->connection_path);
- enm_connection_delete(fd->nm, fd->connection_path);
+ INF("Forget connection: %s (ssid=%s)", conn_path, fd->ssid ?: "(null)");
+
+ /* Optimistic update: remove SSID from local hash immediately so the
+ * forget icon disappears at once without waiting for D-Bus round trips.
+ * The authoritative refresh happens later via the ConnectionRemoved signal. */
+ if (nm->saved_connections && fd->ssid)
+ eina_hash_del_by_key(nm->saved_connections, fd->ssid);
+
+ /* Rebuild all open popups right now, bypassing the 0.5s throttle timer.
+ * This deletes the forget button widget, which frees fd — do not touch
+ * fd after this point. */
+ enm_mod_aps_update_now();
+
+ /* Kick off the actual async delete — ConnectionRemoved signal will
+ * trigger enm_saved_connections_get once NM has committed the removal. */
+ enm_connection_delete(nm, conn_path);
+ eina_stringshare_del(conn_path);
}
static void
@@ -202,6 +223,7 @@ _enm_forget_data_free_cb(void *data, Evas *e EINA_UNUSED,
{
struct _Enm_Forget_Data *fd = data;
eina_stringshare_del(fd->connection_path);
+ eina_stringshare_del(fd->ssid);
free(fd);
}
@@ -256,6 +278,7 @@ _enm_ap_end_new(struct NM_Manager *nm, struct NM_Access_Point *ap, Evas *evas)
evas_object_del(end);
return NULL;
}
+ fd->ssid = eina_stringshare_add(ap->ssid);
evas_object_propagate_events_set(end, EINA_FALSE);
evas_object_event_callback_add(end, EVAS_CALLBACK_MOUSE_UP,
@@ -609,6 +632,27 @@ enm_mod_aps_changed(struct NM_Manager *nm EINA_UNUSED)
ctxt);
}
+void
+enm_mod_aps_update_now(void)
+{
+ E_NM_Module_Context *ctxt;
+ if (!networkmanager_mod) return;
+ ctxt = networkmanager_mod->data;
+ const Eina_List *l;
+ E_NM_Instance *inst;
+
+ /* Cancel any pending throttle timer — we are doing an immediate rebuild */
+ E_FREE_FUNC(ctxt->popup_update_timer, ecore_timer_del);
+
+ if (!ctxt->nm) return;
+
+ EINA_LIST_FOREACH(ctxt->instances, l, inst)
+ {
+ if (!inst->popup) continue;
+ _enm_popup_update(ctxt->nm, inst);
+ }
+}
+
static void
_enm_gadget_setup(E_NM_Instance *inst)
{
diff --git a/src/modules/networkmanager/e_mod_main.h b/src/modules/networkmanager/e_mod_main.h
index 826fff3d6..6231461cf 100644
--- a/src/modules/networkmanager/e_mod_main.h
+++ b/src/modules/networkmanager/e_mod_main.h
@@ -61,6 +61,7 @@ E_API int e_modapi_shutdown(E_Module *m);
E_API int e_modapi_save(E_Module *m);
void enm_popup_del(E_NM_Instance *inst);
+void enm_mod_aps_update_now(void);
const char *e_nm_theme_path(void);
E_NM_Agent *enm_agent_new(Eldbus_Connection *eldbus_conn) EINA_ARG_NONNULL(1);
diff --git a/src/modules/networkmanager/e_networkmanager.c b/src/modules/networkmanager/e_networkmanager.c
index 404c9f4f8..a98343a52 100644
--- a/src/modules/networkmanager/e_networkmanager.c
+++ b/src/modules/networkmanager/e_networkmanager.c
@@ -1078,6 +1078,7 @@ _manager_probe_active_conn(struct NM_Manager *nm,
static void _manager_free(struct NM_Manager *nm);
static struct NM_Manager *_manager_new(void);
+static void _settings_conn_removed_cb(void *data, const Eldbus_Message *msg);
static void
_manager_prop_changed(void *data, const Eldbus_Message *msg)
@@ -1511,12 +1512,21 @@ _connection_delete_cb(void *data, const Eldbus_Message *msg,
{
ERR("Failed to delete connection %s: %s %s",
ctx->path, err_name, err_msg);
+ /* Recover: the optimistic UI removal already hid the forget button.
+ * Refresh saved_connections so the button reappears if the delete
+ * failed (e.g. stale connection path). */
+ enm_saved_connections_get(ctx->nm);
goto done;
}
- INF("Connection %s deleted successfully", ctx->path);
- /* Refresh saved connections to update forget button visibility */
- enm_saved_connections_get(ctx->nm);
+ INF("Connection %s deleted successfully — awaiting ConnectionRemoved signal",
+ ctx->path);
+ /* Do NOT call enm_saved_connections_get here: NM may not have removed the
+ * connection from its internal list yet, so ListConnections would still
+ * return the deleted path and the SSID would be re-added to the hash.
+ * The ConnectionRemoved signal (_settings_conn_removed_cb) fires once NM
+ * has actually removed the object and will trigger the authoritative
+ * refresh. */
done:
eldbus_proxy_unref(ctx->proxy);
@@ -1546,6 +1556,17 @@ enm_connection_delete(struct NM_Manager *nm, const char *connection_path)
/* ctx, proxy, and obj are freed/unref'd inside _connection_delete_cb */
}
+/* Called when NM removes a saved connection — refresh the hash so the
+ * forget button disappears without a visible race window. */
+static void
+_settings_conn_removed_cb(void *data, const Eldbus_Message *msg EINA_UNUSED)
+{
+ struct NM_Manager *nm = data;
+ if (!nm) return;
+ INF("ConnectionRemoved signal: refreshing saved connections");
+ enm_saved_connections_get(nm);
+}
+
static struct NM_Manager *
_manager_new(void)
{
@@ -1578,6 +1599,16 @@ _manager_new(void)
eldbus_proxy_call(nm->proxy, "GetDevices",
_manager_get_devices_cb, nm, -1, "");
+ /* Persistent Settings object for ConnectionRemoved signal.
+ * This avoids the race in _connection_delete_cb where ListConnections
+ * may still return the just-deleted path before NM's internal list is
+ * updated. Instead we react to the authoritative signal. */
+ nm->settings_obj = eldbus_object_get(conn, NM_BUS_NAME, NM_SETTINGS_PATH);
+ nm->settings_proxy = eldbus_proxy_get(nm->settings_obj, NM_IFACE_SETTINGS);
+ nm->conn_removed_handler =
+ eldbus_proxy_signal_handler_add(nm->settings_proxy, "ConnectionRemoved",
+ _settings_conn_removed_cb, nm);
+
return nm;
}
@@ -1614,6 +1645,23 @@ _manager_free(struct NM_Manager *nm)
eina_stringshare_del(nm->active_ap_path);
eina_stringshare_del(nm->active_connection_path);
+ /* Tear down the Settings signal subscription */
+ if (nm->conn_removed_handler)
+ {
+ eldbus_signal_handler_del(nm->conn_removed_handler);
+ nm->conn_removed_handler = NULL;
+ }
+ if (nm->settings_proxy)
+ {
+ eldbus_proxy_unref(nm->settings_proxy);
+ nm->settings_proxy = NULL;
+ }
+ if (nm->settings_obj)
+ {
+ eldbus_object_unref(nm->settings_obj);
+ nm->settings_obj = 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 152378184..b128f1abd 100644
--- a/src/modules/networkmanager/e_networkmanager.h
+++ b/src/modules/networkmanager/e_networkmanager.h
@@ -130,6 +130,12 @@ struct NM_Manager
Eina_Hash *saved_connections;
int saved_conn_pending; /* outstanding GetSettings calls */
+ /* Long-lived Settings object/proxy for ConnectionRemoved signal subscription.
+ * Created in _manager_new, freed in _manager_free. */
+ Eldbus_Proxy *settings_proxy;
+ Eldbus_Object *settings_obj;
+ Eldbus_Signal_Handler *conn_removed_handler;
+
/* Generation counter incremented each time a new batch of active-connection
* probes is started. Each probe captures the generation at creation time
* and discards its result in the callback if the generation has advanced,
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
