From 0280bbae3b37b36419a4894134c1c78127e049ab Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Thu, 2 Jul 2026 16:41:05 +0000 Subject: [PATCH 1/7] time: fix GSource leak and UAF in timezone poll Introduce a timezone poll session ID to validate timezone load results. This ensures that when a pending timezone load completes, we discard the result if a new poll session has been started (e.g. due to registration state changes) or if polling has been stopped. This prevents leaking GSource timeout sources and a subsequent Use-After-Free if the modem is hot-unplugged while a poll is pending. https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1067 Signed-off-by: Aleksander Morgado --- src/mm-iface-modem-time.c | 46 +++++++++++++++++++++++++++------------ 1 file changed, 32 insertions(+), 14 deletions(-) diff --git a/src/mm-iface-modem-time.c b/src/mm-iface-modem-time.c index 4f9e7f3f..52ca139a 100644 --- a/src/mm-iface-modem-time.c +++ b/src/mm-iface-modem-time.c @@ -153,6 +153,7 @@ typedef struct { MMModemState state; guint network_timezone_poll_id; guint network_timezone_poll_retries; + guint session_id; } NetworkTimezoneContext; static void @@ -191,23 +192,34 @@ update_network_timezone_dictionary (MMIfaceModemTime *self, static void load_network_timezone_ready (MMIfaceModemTime *self, - GAsyncResult *res) + GAsyncResult *res, + gpointer user_data) { - g_autoptr(GError) error = NULL; - MMNetworkTimezone *tz; + g_autoptr(GError) error = NULL; + g_autoptr(MMNetworkTimezone) tz = NULL; + NetworkTimezoneContext *ctx; + guint session_id; - /* Finish the async operation */ + session_id = GPOINTER_TO_UINT (user_data); + + /* Finish the async operation first to avoid resource leaks */ tz = MM_IFACE_MODEM_TIME_GET_IFACE (self)->load_network_timezone_finish (self, res, &error); + + /* Note: may be NULL if the polling has been removed while processing the async operation */ + ctx = (NetworkTimezoneContext *) g_object_get_qdata (G_OBJECT (self), network_timezone_context_quark); + if (!ctx) + return; + + /* Check if this is the result of the current poll session */ + if (ctx->session_id != session_id) { + mm_obj_dbg (self, "discarding network timezone load result from old session (session ID %u, current %u)", + session_id, ctx->session_id); + return; + } + if (!tz) { - NetworkTimezoneContext *ctx; - mm_obj_dbg (self, "couldn't load network timezone: %s", error->message); - /* Note: may be NULL if the polling has been removed while processing the async operation */ - ctx = (NetworkTimezoneContext *) g_object_get_qdata (G_OBJECT (self), network_timezone_context_quark); - if (!ctx) - return; - /* Retry? */ ctx->network_timezone_poll_retries--; @@ -227,7 +239,6 @@ load_network_timezone_ready (MMIfaceModemTime *self, /* Got final result properly, update the property in the skeleton */ update_network_timezone_dictionary (self, tz); - g_object_unref (tz); } static gboolean @@ -241,7 +252,7 @@ network_timezone_poll_cb (MMIfaceModemTime *self) MM_IFACE_MODEM_TIME_GET_IFACE (self)->load_network_timezone ( self, (GAsyncReadyCallback)load_network_timezone_ready, - NULL); + GUINT_TO_POINTER (ctx->session_id)); return G_SOURCE_REMOVE; } @@ -253,7 +264,13 @@ start_network_timezone_poll (MMIfaceModemTime *self) ctx = (NetworkTimezoneContext *) g_object_get_qdata (G_OBJECT (self), network_timezone_context_quark); - mm_obj_dbg (self, "network timezone polling started"); + if (ctx->network_timezone_poll_id) { + g_source_remove (ctx->network_timezone_poll_id); + ctx->network_timezone_poll_id = 0; + } + ctx->session_id++; + + mm_obj_dbg (self, "network timezone polling started (session ID %u)", ctx->session_id); ctx->network_timezone_poll_retries = NETWORK_TIMEZONE_POLL_RETRIES; ctx->network_timezone_poll_id = g_timeout_add_seconds (NETWORK_TIMEZONE_POLL_INTERVAL_SEC, (GSourceFunc)network_timezone_poll_cb, self); } @@ -270,6 +287,7 @@ stop_network_timezone_poll (MMIfaceModemTime *self) g_source_remove (ctx->network_timezone_poll_id); ctx->network_timezone_poll_id = 0; } + ctx->session_id++; } static void From 2b1aaca4a0f729f8a54f162ff9dc299c82162bed Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Thu, 2 Jul 2026 16:49:37 +0000 Subject: [PATCH 2/7] ifaces: fix UAFs in signal handlers when hot-unplugging Use g_signal_connect_object() to connect to child list/object signals in the 3GPP, Messaging, Voice, and Cell Broadcast interfaces. Passing the raw D-Bus skeleton pointer as user_data to these signals can lead to a Use-After-Free if the skeleton is disposed of (e.g. on hot-unplug) while the parent object or child list survives. By using g_signal_connect_object(), the signal handlers are automatically disconnected when the D-Bus skeleton is destroyed. https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1068 Signed-off-by: Aleksander Morgado --- src/mm-iface-modem-3gpp.c | 9 +++++---- src/mm-iface-modem-cell-broadcast.c | 18 ++++++++++-------- src/mm-iface-modem-messaging.c | 18 ++++++++++-------- src/mm-iface-modem-voice.c | 18 ++++++++++-------- 4 files changed, 35 insertions(+), 28 deletions(-) diff --git a/src/mm-iface-modem-3gpp.c b/src/mm-iface-modem-3gpp.c index b72ae0e4..9273f83f 100644 --- a/src/mm-iface-modem-3gpp.c +++ b/src/mm-iface-modem-3gpp.c @@ -3713,10 +3713,11 @@ load_enabled_facility_locks_ready (MMIfaceModem3gpp *self, g_object_get (self, MM_IFACE_MODEM_SIM, &sim, NULL); if (sim) - g_signal_connect (sim, - MM_BASE_SIM_PIN_LOCK_ENABLED, - G_CALLBACK (sim_pin_lock_enabled_cb), - ctx->skeleton); + g_signal_connect_object (sim, + MM_BASE_SIM_PIN_LOCK_ENABLED, + G_CALLBACK (sim_pin_lock_enabled_cb), + ctx->skeleton, + 0); } /* Go on to next step */ diff --git a/src/mm-iface-modem-cell-broadcast.c b/src/mm-iface-modem-cell-broadcast.c index 32a77535..0776c1c4 100644 --- a/src/mm-iface-modem-cell-broadcast.c +++ b/src/mm-iface-modem-cell-broadcast.c @@ -636,14 +636,16 @@ interface_enabling_step (GTask *task) NULL); /* Connect to list's signals */ - g_signal_connect (list, - MM_CBM_ADDED, - G_CALLBACK (cbm_added), - ctx->skeleton); - g_signal_connect (list, - MM_CBM_DELETED, - G_CALLBACK (cbm_deleted), - ctx->skeleton); + g_signal_connect_object (list, + MM_CBM_ADDED, + G_CALLBACK (cbm_added), + ctx->skeleton, + 0); + g_signal_connect_object (list, + MM_CBM_DELETED, + G_CALLBACK (cbm_deleted), + ctx->skeleton, + 0); ctx->step++; } /* fall through */ diff --git a/src/mm-iface-modem-messaging.c b/src/mm-iface-modem-messaging.c index 93cd0ae1..be085bfc 100644 --- a/src/mm-iface-modem-messaging.c +++ b/src/mm-iface-modem-messaging.c @@ -1026,14 +1026,16 @@ interface_enabling_step (GTask *task) NULL); /* Connect to list's signals */ - g_signal_connect (list, - MM_SMS_ADDED, - G_CALLBACK (sms_added), - ctx->skeleton); - g_signal_connect (list, - MM_SMS_DELETED, - G_CALLBACK (sms_deleted), - ctx->skeleton); + g_signal_connect_object (list, + MM_SMS_ADDED, + G_CALLBACK (sms_added), + ctx->skeleton, + 0); + g_signal_connect_object (list, + MM_SMS_DELETED, + G_CALLBACK (sms_deleted), + ctx->skeleton, + 0); g_object_unref (list); diff --git a/src/mm-iface-modem-voice.c b/src/mm-iface-modem-voice.c index b3907e53..d6ea4aae 100644 --- a/src/mm-iface-modem-voice.c +++ b/src/mm-iface-modem-voice.c @@ -3019,14 +3019,16 @@ interface_initialization_step (GTask *task) NULL); /* Connect to list's signals */ - g_signal_connect (list, - MM_CALL_ADDED, - G_CALLBACK (call_added), - ctx->skeleton); - g_signal_connect (list, - MM_CALL_DELETED, - G_CALLBACK (call_deleted), - ctx->skeleton); + g_signal_connect_object (list, + MM_CALL_ADDED, + G_CALLBACK (call_added), + ctx->skeleton, + 0); + g_signal_connect_object (list, + MM_CALL_DELETED, + G_CALLBACK (call_deleted), + ctx->skeleton, + 0); /* Setup monitoring for in-call event handling */ g_signal_connect (list, From 46a91399ca18e27b26e6ffc35f0b4bdf5cb14802 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Thu, 2 Jul 2026 17:00:13 +0000 Subject: [PATCH 3/7] shared-qmi: fix UAF on duplicate list-configs indications Disconnect the 'list-configs' indication handler immediately inside the callback. If the modem sends a duplicate indication before the whole config load sequence completes, it could overwrite the stored 'get-config-info' signal handler ID, orphaning the old handler and leading to a Use-After-Free when the GTask is completed and the orphaned handler eventually fires. https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1071 Signed-off-by: Aleksander Morgado --- src/mm-shared-qmi.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/mm-shared-qmi.c b/src/mm-shared-qmi.c index d93698a6..f4193b60 100644 --- a/src/mm-shared-qmi.c +++ b/src/mm-shared-qmi.c @@ -3111,6 +3111,12 @@ list_configs_indication (QmiClientPdc *client, self = g_task_get_source_object (task); ctx = g_task_get_task_data (task); + /* Disconnect list-configs indication right away to avoid re-entry */ + if (ctx->list_configs_indication_id) { + g_signal_handler_disconnect (ctx->client, ctx->list_configs_indication_id); + ctx->list_configs_indication_id = 0; + } + if (!qmi_indication_pdc_list_configs_output_get_indication_result (output, &error_code, &error)) { load_carrier_config_abort (task, error); return; From 166d6c5bc8da3cc822bebb0e9bb6f24ea0a77aed Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Thu, 2 Jul 2026 17:08:11 +0000 Subject: [PATCH 4/7] quectel: fix UAF in RDY URC handler Defer the validity change (which triggers modem reprobe/removal) to an idle callback in rdy_handler(). Calling mm_base_modem_set_valid(FALSE) synchronously from the URC handler triggers the immediate destruction of the modem object while the serial port is still iterating the unsolicited message handlers. Deferring the operation to an idle callback and taking a reference to the modem ensures the modem object is kept alive until parse_unsolicited() has returned. https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1072 Signed-off-by: Aleksander Morgado --- src/plugins/quectel/mm-shared-quectel.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/plugins/quectel/mm-shared-quectel.c b/src/plugins/quectel/mm-shared-quectel.c index 1bae73e4..a7bc1341 100644 --- a/src/plugins/quectel/mm-shared-quectel.c +++ b/src/plugins/quectel/mm-shared-quectel.c @@ -152,6 +152,14 @@ mm_shared_quectel_create_wwan_port (MMBaseModem *self, /*****************************************************************************/ /* RDY unsolicited event handler */ +static gboolean +rdy_handler_idle (MMBroadbandModem *self) +{ + mm_base_modem_set_reprobe (MM_BASE_MODEM (self), TRUE); + mm_base_modem_set_valid (MM_BASE_MODEM (self), FALSE); + return G_SOURCE_REMOVE; +} + static void rdy_handler (MMPortSerialAt *port, GMatchInfo *match_info, @@ -162,8 +170,10 @@ rdy_handler (MMPortSerialAt *port, * re-synchronize modem and ModemManager states by re-probing. */ mm_obj_warn (self, "modem reset detected, triggering reprobe"); - mm_base_modem_set_reprobe (MM_BASE_MODEM (self), TRUE); - mm_base_modem_set_valid (MM_BASE_MODEM (self), FALSE); + g_idle_add_full (G_PRIORITY_DEFAULT_IDLE, + (GSourceFunc) rdy_handler_idle, + g_object_ref (self), + g_object_unref); } /*****************************************************************************/ From c3e7ffaae04ae6ff9e810093ca1c102a5572e6be Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Thu, 2 Jul 2026 17:39:30 +0000 Subject: [PATCH 5/7] plugins: fix GError double-frees when returning errors from tasks Use g_steal_pointer() when passing g_autoptr(GError) to g_task_return_error() to prevent double-free by the automatic cleanup. GTask takes ownership of the passed GError. If we use g_autoptr(GError) locally, it will also call g_error_free() when the function returns, causing a double-free / heap corruption. Stealing the pointer clears the local variable and prevents the automatic cleanup. https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1073 Signed-off-by: Aleksander Morgado --- src/plugins/cinterion/mm-broadband-modem-cinterion.c | 8 ++++---- src/plugins/cinterion/mm-shared-cinterion.c | 4 ++-- src/plugins/fibocom/mm-shared-fibocom.c | 2 +- src/plugins/foxconn/mm-broadband-modem-mbim-foxconn.c | 2 +- src/plugins/quectel/mm-broadband-modem-quectel.c | 2 +- src/plugins/quectel/mm-shared-quectel.c | 2 +- 6 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/plugins/cinterion/mm-broadband-modem-cinterion.c b/src/plugins/cinterion/mm-broadband-modem-cinterion.c index 3d70cf50..e0cd82d4 100644 --- a/src/plugins/cinterion/mm-broadband-modem-cinterion.c +++ b/src/plugins/cinterion/mm-broadband-modem-cinterion.c @@ -1177,7 +1177,7 @@ modem_3gpp_register_in_network (MMIfaceModem3gpp *_self, operator_code, &command, &error)) { - g_task_return_error (task, error); + g_task_return_error (task, g_steal_pointer (&error)); g_object_unref (task); return; } @@ -1723,14 +1723,14 @@ ws46_query_ready (MMBaseModem *self, response = mm_base_modem_at_command_finish (self, res, &error); if (!response) { - g_task_return_error (task, error); + g_task_return_error (task, g_steal_pointer (&error)); g_object_unref (task); return; } if (!mm_cinterion_parse_ws46_response (response, &(result->allowed), &error)) { - g_task_return_error (task, error); + g_task_return_error (task, g_steal_pointer (&error)); g_object_unref (task); return; } @@ -1882,7 +1882,7 @@ cops_set_current_modes (MMBroadbandModemCinterion *self, operator_id, &command, &error)) { - g_task_return_error (task, error); + g_task_return_error (task, g_steal_pointer (&error)); g_object_unref (task); return; } diff --git a/src/plugins/cinterion/mm-shared-cinterion.c b/src/plugins/cinterion/mm-shared-cinterion.c index aa9b6208..3a703a22 100644 --- a/src/plugins/cinterion/mm-shared-cinterion.c +++ b/src/plugins/cinterion/mm-shared-cinterion.c @@ -266,7 +266,7 @@ parent_load_update_settings_ready (MMIfaceModemFirmware *self, priv = get_private (MM_SHARED_CINTERION (self)); update_settings = priv->iface_modem_firmware_parent->load_update_settings_finish (self, res, &error); if (error) { - g_task_return_error (task, error); + g_task_return_error (task, g_steal_pointer (&error)); g_object_unref (task); return; } @@ -276,7 +276,7 @@ parent_load_update_settings_ready (MMIfaceModemFirmware *self, ids = mm_iface_firmware_build_generic_device_ids (MM_IFACE_MODEM_FIRMWARE (self), &error); if (error) { mm_obj_warn (self, "failed to build generic device ids: %s", error->message); - g_task_return_error (task, error); + g_task_return_error (task, g_steal_pointer (&error)); g_object_unref (task); return; } diff --git a/src/plugins/fibocom/mm-shared-fibocom.c b/src/plugins/fibocom/mm-shared-fibocom.c index 1f9559c4..9259d080 100644 --- a/src/plugins/fibocom/mm-shared-fibocom.c +++ b/src/plugins/fibocom/mm-shared-fibocom.c @@ -343,7 +343,7 @@ parent_load_update_settings_ready (MMIfaceModemFirmware *self, priv = get_private (MM_SHARED_FIBOCOM (self)); update_settings = priv->iface_modem_firmware_parent->load_update_settings_finish (self, res, &error); if (error) { - g_task_return_error (task, error); + g_task_return_error (task, g_steal_pointer (&error)); g_object_unref (task); return; } diff --git a/src/plugins/foxconn/mm-broadband-modem-mbim-foxconn.c b/src/plugins/foxconn/mm-broadband-modem-mbim-foxconn.c index 9e32df3b..21138c6a 100644 --- a/src/plugins/foxconn/mm-broadband-modem-mbim-foxconn.c +++ b/src/plugins/foxconn/mm-broadband-modem-mbim-foxconn.c @@ -276,7 +276,7 @@ parent_load_update_settings_ready (MMIfaceModemFirmware *self, update_settings = iface_modem_firmware_parent->load_update_settings_finish (self, res, &error); if (error) { - g_task_return_error (task, error); + g_task_return_error (task, g_steal_pointer (&error)); g_object_unref (task); return; } diff --git a/src/plugins/quectel/mm-broadband-modem-quectel.c b/src/plugins/quectel/mm-broadband-modem-quectel.c index 072e291b..dbf8c427 100644 --- a/src/plugins/quectel/mm-broadband-modem-quectel.c +++ b/src/plugins/quectel/mm-broadband-modem-quectel.c @@ -212,7 +212,7 @@ power_off_ready (MMBroadbandModemQuectel *self, */ if (!mm_base_modem_at_command_finish (MM_BASE_MODEM (self), res, &error)) { - g_task_return_error (task, error); + g_task_return_error (task, g_steal_pointer (&error)); g_object_unref (task); return; } diff --git a/src/plugins/quectel/mm-shared-quectel.c b/src/plugins/quectel/mm-shared-quectel.c index a7bc1341..9d36583f 100644 --- a/src/plugins/quectel/mm-shared-quectel.c +++ b/src/plugins/quectel/mm-shared-quectel.c @@ -485,7 +485,7 @@ parent_load_update_settings_ready (MMIfaceModemFirmware *self, priv = get_private (MM_SHARED_QUECTEL (self)); update_settings = priv->iface_modem_firmware_parent->load_update_settings_finish (self, res, &error); if (error) { - g_task_return_error (task, error); + g_task_return_error (task, g_steal_pointer (&error)); g_object_unref (task); return; } From db72a182c108c91796ee10d323559309b1a06ef5 Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Thu, 2 Jul 2026 17:53:40 +0000 Subject: [PATCH 6/7] port-serial-at: fix crash if URC handler is cleared mid-dispatch Update parse_unsolicited() to re-check handler->callback on each iteration of the match dispatch loop. If a URC handler callback clears itself during dispatch (e.g. to stop listening), subsequent matches in the same buffer would attempt to call the NULL pointer, causing a crash. https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1074 Signed-off-by: Aleksander Morgado --- src/mm-port-serial-at.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mm-port-serial-at.c b/src/mm-port-serial-at.c index ca2cc540..267ebdd5 100644 --- a/src/mm-port-serial-at.c +++ b/src/mm-port-serial-at.c @@ -241,7 +241,7 @@ parse_unsolicited (MMPortSerial *port, GByteArray *response) response->len, 0, 0, &match_info, NULL); if (handler->callback) { - while (g_match_info_matches (match_info)) { + while (g_match_info_matches (match_info) && handler->callback) { handler->callback (self, match_info, handler->user_data); g_match_info_next (match_info, NULL); } From 9642ed9ab5c8f60228b5ce96958b775eb1ecd3fc Mon Sep 17 00:00:00 2001 From: Aleksander Morgado Date: Thu, 2 Jul 2026 17:53:47 +0000 Subject: [PATCH 7/7] ublox: fix UAF in custom init ready URC handler Clear the unsolicited message handler in ready_received() before completing the task. Otherwise, if the modem sends subsequent READY URCs, the handler would fire again using the freed GTask pointer. https://gitlab.freedesktop.org/mobile-broadband/ModemManager/-/work_items/1074 Signed-off-by: Aleksander Morgado --- src/plugins/ublox/mm-plugin-ublox.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/plugins/ublox/mm-plugin-ublox.c b/src/plugins/ublox/mm-plugin-ublox.c index 7921b02f..66eda597 100644 --- a/src/plugins/ublox/mm-plugin-ublox.c +++ b/src/plugins/ublox/mm-plugin-ublox.c @@ -113,6 +113,9 @@ ready_received (MMPortSerialAt *port, g_source_remove (ctx->timeout_id); ctx->timeout_id = 0; + mm_port_serial_at_add_unsolicited_msg_handler (port, ctx->ready_regex, + NULL, NULL, NULL); + mm_obj_dbg (probe, "received READY: port is AT"); /* Flag as an AT port right away */