From de4f4e870dae3aae3dd77953b9e47a7cef7f5f90 Mon Sep 17 00:00:00 2001 From: Tomas Korbar Date: Thu, 13 Mar 2025 12:31:14 +0100 Subject: [PATCH 1/2] dns: Fix invalid memory access on Dnsconfd DBUS error DBus errors were not properly handled after DBus calls and that caused SIGSEGV. Now they are checked. Fixes #1738 Fixes: b8714e86e4e7 ('dns: introduce configuration_serial support to the dnsconfd plugin') (cherry picked from commit 4ad20787bbeb53559e96bdd74e06b8267a9d287b) --- src/core/dns/nm-dns-dnsconfd.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/core/dns/nm-dns-dnsconfd.c b/src/core/dns/nm-dns-dnsconfd.c index 63b3060f3d..c994ec8bc9 100644 --- a/src/core/dns/nm-dns-dnsconfd.c +++ b/src/core/dns/nm-dns-dnsconfd.c @@ -132,6 +132,13 @@ dnsconfd_serial_retrieval_done(GObject *source_object, GAsyncResult *res, gpoint self = user_data; priv = NM_DNS_DNSCONFD_GET_PRIVATE(self); + if (!response) { + _LOGW("dnsconfd serial retrieval failed: %s", error->message); + priv->plugin_state = DNSCONFD_PLUGIN_IDLE; + _nm_dns_plugin_update_pending_maybe_changed(NM_DNS_PLUGIN(self)); + return; + } + nm_clear_g_cancellable(&priv->serial_cancellable); g_variant_get(response, "(v)", &new_serial_variant); @@ -201,8 +208,12 @@ dnsconfd_update_done(GObject *source_object, GAsyncResult *res, gpointer user_da nm_clear_g_cancellable(&priv->update_cancellable); - if (!response) + if (!response) { _LOGW("dnsconfd update failed: %s", error->message); + priv->plugin_state = DNSCONFD_PLUGIN_IDLE; + _nm_dns_plugin_update_pending_maybe_changed(NM_DNS_PLUGIN(self)); + return; + } /* By using &s we will get pointer to char data contained * in variant and thus no freing of dnsconfd_message is required */ -- 2.49.0 From 873adc4dc04088542b107ebd6aa2289a4c4f6df9 Mon Sep 17 00:00:00 2001 From: Tomas Korbar Date: Thu, 13 Mar 2025 12:34:09 +0100 Subject: [PATCH 2/2] dns: Refactor changing of Dnsconfd plugin state (cherry picked from commit 7ba27f7a13afaa8a55e662cd1857d480c52a3a85) --- src/core/dns/nm-dns-dnsconfd.c | 45 ++++++++++++++++------------------ 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/src/core/dns/nm-dns-dnsconfd.c b/src/core/dns/nm-dns-dnsconfd.c index c994ec8bc9..c17fb9cf44 100644 --- a/src/core/dns/nm-dns-dnsconfd.c +++ b/src/core/dns/nm-dns-dnsconfd.c @@ -71,6 +71,15 @@ typedef enum { /*****************************************************************************/ +static void +dnsconfd_change_plugin_state(NMDnsDnsconfd *self, DnsconfdPluginState new_state) +{ + NMDnsDnsconfdPrivate *priv = NM_DNS_DNSCONFD_GET_PRIVATE(self); + + priv->plugin_state = new_state; + _nm_dns_plugin_update_pending_maybe_changed(NM_DNS_PLUGIN(self)); +} + static void dnsconfd_serial_changed(NMDnsDnsconfd *self, guint new_serial) { @@ -78,12 +87,10 @@ dnsconfd_serial_changed(NMDnsDnsconfd *self, guint new_serial) priv->present_configuration_serial = new_serial; if (priv->plugin_state == DNSCONFD_PLUGIN_WAIT_SERIAL && priv->awaited_configuration_serial == new_serial) { - priv->plugin_state = DNSCONFD_PLUGIN_IDLE; + dnsconfd_change_plugin_state(self, DNSCONFD_PLUGIN_IDLE); /* Update finished, serials match */ _LOGT("serials match, update finished"); } - - _nm_dns_plugin_update_pending_maybe_changed(NM_DNS_PLUGIN(self)); } static void @@ -134,8 +141,7 @@ dnsconfd_serial_retrieval_done(GObject *source_object, GAsyncResult *res, gpoint if (!response) { _LOGW("dnsconfd serial retrieval failed: %s", error->message); - priv->plugin_state = DNSCONFD_PLUGIN_IDLE; - _nm_dns_plugin_update_pending_maybe_changed(NM_DNS_PLUGIN(self)); + dnsconfd_change_plugin_state(self, DNSCONFD_PLUGIN_IDLE); return; } @@ -210,8 +216,7 @@ dnsconfd_update_done(GObject *source_object, GAsyncResult *res, gpointer user_da if (!response) { _LOGW("dnsconfd update failed: %s", error->message); - priv->plugin_state = DNSCONFD_PLUGIN_IDLE; - _nm_dns_plugin_update_pending_maybe_changed(NM_DNS_PLUGIN(self)); + dnsconfd_change_plugin_state(self, DNSCONFD_PLUGIN_IDLE); return; } @@ -221,8 +226,7 @@ dnsconfd_update_done(GObject *source_object, GAsyncResult *res, gpointer user_da if (!awaited_serial) { _LOGW("dnsconfd refused update: %s", dnsconfd_message); - priv->plugin_state = DNSCONFD_PLUGIN_IDLE; - _nm_dns_plugin_update_pending_maybe_changed(NM_DNS_PLUGIN(self)); + dnsconfd_change_plugin_state(self, DNSCONFD_PLUGIN_IDLE); return; } @@ -231,14 +235,12 @@ dnsconfd_update_done(GObject *source_object, GAsyncResult *res, gpointer user_da if (priv->awaited_configuration_serial == priv->present_configuration_serial) { /* Serials match, update finished */ - priv->plugin_state = DNSCONFD_PLUGIN_IDLE; + dnsconfd_change_plugin_state(self, DNSCONFD_PLUGIN_IDLE); _LOGT("after update serials match"); } else { - priv->plugin_state = DNSCONFD_PLUGIN_WAIT_SERIAL; + dnsconfd_change_plugin_state(self, DNSCONFD_PLUGIN_WAIT_SERIAL); _LOGT("after update serials don't match, waiting"); } - - _nm_dns_plugin_update_pending_maybe_changed(NM_DNS_PLUGIN(self)); } static gboolean @@ -489,8 +491,7 @@ name_owner_changed(NMDnsDnsconfd *self, const char *name_owner) || priv->plugin_state == DNSCONFD_PLUGIN_WAIT_SERIAL) { /* We were waiting for either serial or confirmation of update and name * disappeared, thus we need to retransmit */ - priv->plugin_state = DNSCONFD_PLUGIN_WAIT_CONNECT; - _nm_dns_plugin_update_pending_maybe_changed(NM_DNS_PLUGIN(self)); + dnsconfd_change_plugin_state(self, DNSCONFD_PLUGIN_WAIT_CONNECT); } return; } @@ -501,15 +502,13 @@ name_owner_changed(NMDnsDnsconfd *self, const char *name_owner) if (!subscribe_serial(self)) { /* This means that in time between new name and subscribe serial call * we lost the name again thus wait again */ - priv->plugin_state = DNSCONFD_PLUGIN_WAIT_CONNECT; + dnsconfd_change_plugin_state(self, DNSCONFD_PLUGIN_WAIT_CONNECT); _LOGT("subscription failed, waiting to connect"); } else { - priv->plugin_state = DNSCONFD_PLUGIN_WAIT_UPDATE_DONE; + dnsconfd_change_plugin_state(self, DNSCONFD_PLUGIN_WAIT_UPDATE_DONE); _LOGT("sending update and waiting for its finish"); send_dnsconfd_update(self); } - - _nm_dns_plugin_update_pending_maybe_changed(NM_DNS_PLUGIN(self)); } static void @@ -706,18 +705,16 @@ update(NMDnsPlugin *plugin, /* We need to consider only whether we are connected, because newer update call * overrides the old one */ if (all_connected == CONNECTION_FAIL) { - priv->plugin_state = DNSCONFD_PLUGIN_IDLE; + dnsconfd_change_plugin_state(self, DNSCONFD_PLUGIN_IDLE); _LOGT("failed to connect"); } else if (all_connected == CONNECTION_WAIT) { - priv->plugin_state = DNSCONFD_PLUGIN_WAIT_CONNECT; + dnsconfd_change_plugin_state(self, DNSCONFD_PLUGIN_WAIT_CONNECT); _LOGT("not connected, waiting to connect"); } else { - priv->plugin_state = DNSCONFD_PLUGIN_WAIT_UPDATE_DONE; + dnsconfd_change_plugin_state(self, DNSCONFD_PLUGIN_WAIT_UPDATE_DONE); _LOGT("connected, waiting for update to finish"); } - _nm_dns_plugin_update_pending_maybe_changed(plugin); - if (all_connected == CONNECTION_FAIL) { nm_utils_error_set(error, NM_UTILS_ERROR_UNKNOWN, -- 2.49.0