core: merge branch 'th/save-hostname-fixes'

https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/1238

3 files changed, 183 insertions, 55 deletions

diff --git a/src/core/nm-hostname-manager.c b/src/core/nm-hostname-manager.c
index 64c2531e61..be1ba77be5 100644
--- a/src/core/nm-hostname-manager.c
+++ b/src/core/nm-hostname-manager.c
@@ -239,7 +239,7 @@ _set_hostname_read_file(NMHostnameManager *self)
 #if defined(HOSTNAME_PERSIST_GENTOO)
     hostname = read_hostname_gentoo(HOSTNAME_FILE);
 #elif defined(HOSTNAME_PERSIST_SLACKWARE)
-    hostname     = read_hostname_slackware(HOSTNAME_FILE);
+    hostname = read_hostname_slackware(HOSTNAME_FILE);
 #else
     if (g_file_get_contents(HOSTNAME_FILE, &hostname, NULL, NULL))
         g_strchomp(hostname);
@@ -321,11 +321,31 @@ nm_hostname_manager_get_transient_hostname(NMHostnameManager *self, char **hostn
     return TRUE;
 }
 
-gboolean
-nm_hostname_manager_write_hostname(NMHostnameManager *self, const char *hostname)
+/*****************************************************************************/
+
+static void
+_write_hostname_dbus_cb(GObject *source, GAsyncResult *result, gpointer user_data)
+{
+    gs_unref_object GTask     *task  = G_TASK(user_data);
+    gs_unref_variant GVariant *res   = NULL;
+    GError                    *error = NULL;
+
+    res = g_dbus_proxy_call_finish(G_DBUS_PROXY(source), result, &error);
+    if (!res) {
+        g_task_return_error(task, error);
+        return;
+    }
+    g_task_return_boolean(task, TRUE);
+}
+
+static void
+_write_hostname_on_idle_cb(gpointer user_data, GCancellable *cancellable)
 {
+    gs_unref_object GTask     *task = G_TASK(user_data);
+    NMHostnameManager         *self;
     NMHostnameManagerPrivate  *priv;
-    char                      *hostname_eol;
+    const char                *hostname;
+    gs_free char              *hostname_eol = NULL;
     gboolean                   ret;
     gs_free_error GError      *error     = NULL;
     const char                *file      = HOSTNAME_FILE;
@@ -337,23 +357,15 @@ nm_hostname_manager_write_hostname(NMHostnameManager *self, const char *hostname
     char    *fcon_prev    = NULL;
 #endif
 
-    g_return_val_if_fail(NM_IS_HOSTNAME_MANAGER(self), FALSE);
+    if (g_task_return_error_if_cancelled(task))
+        return;
 
+    self = g_task_get_source_object(task);
     priv = NM_HOSTNAME_MANAGER_GET_PRIVATE(self);
 
-    if (priv->hostnamed_proxy) {
-        var = g_dbus_proxy_call_sync(priv->hostnamed_proxy,
-                                     "SetStaticHostname",
-                                     g_variant_new("(sb)", hostname, FALSE),
-                                     G_DBUS_CALL_FLAGS_NONE,
-                                     -1,
-                                     NULL,
-                                     &error);
-        if (error)
-            _LOGW("could not set hostname: %s", error->message);
-
-        return !error;
-    }
+    nm_assert(!priv->hostnamed_proxy);
+
+    hostname = g_task_get_task_data(task);
 
     /* If the hostname file is a symbolic link, follow it to find where the
      * real file is located, otherwise g_file_set_contents will attempt to
@@ -363,13 +375,15 @@ nm_hostname_manager_write_hostname(NMHostnameManager *self, const char *hostname
         && (link_path = nm_utils_read_link_absolute(file, NULL)))
         file = link_path;
 
+    if (hostname) {
 #if defined(HOSTNAME_PERSIST_GENTOO)
-    hostname_eol = g_strdup_printf("#Generated by NetworkManager\n"
-                                   "hostname=\"%s\"\n",
-                                   hostname);
+        hostname_eol = g_strdup_printf("#Generated by NetworkManager\n"
+                                       "hostname=\"%s\"\n",
+                                       hostname);
 #else
-    hostname_eol = g_strdup_printf("%s\n", hostname);
+        hostname_eol = g_strdup_printf("%s\n", hostname);
 #endif
+    }
 
 #if HAVE_SELINUX
     /* Get default context for hostname file and set it for fscreate */
@@ -396,7 +410,7 @@ nm_hostname_manager_write_hostname(NMHostnameManager *self, const char *hostname
     }
 #endif
 
-    ret = g_file_set_contents(file, hostname_eol, -1, &error);
+    ret = g_file_set_contents(file, hostname_eol ?: "", -1, &error);
 
 #if HAVE_SELINUX
     /* Restore previous context and cleanup */
@@ -406,14 +420,63 @@ nm_hostname_manager_write_hostname(NMHostnameManager *self, const char *hostname
         freecon(fcon_prev);
 #endif
 
-    g_free(hostname_eol);
-
     if (!ret) {
-        _LOGW("could not save hostname to %s: %s", file, error->message);
-        return FALSE;
+        g_task_return_new_error(task,
+                                NM_UTILS_ERROR,
+                                NM_UTILS_ERROR_UNKNOWN,
+                                "could not save hostname to %s: %s",
+                                file,
+                                error->message);
+        return;
     }
 
-    return TRUE;
+    g_task_return_boolean(task, TRUE);
+}
+
+void
+nm_hostname_manager_write_hostname(NMHostnameManager  *self,
+                                   const char         *hostname,
+                                   GCancellable       *cancellable,
+                                   GAsyncReadyCallback callback,
+                                   gpointer            user_data)
+{
+    NMHostnameManagerPrivate *priv;
+    GTask                    *task;
+
+    g_return_if_fail(NM_IS_HOSTNAME_MANAGER(self));
+
+    priv = NM_HOSTNAME_MANAGER_GET_PRIVATE(self);
+
+    task =
+        nm_g_task_new(self, cancellable, nm_hostname_manager_write_hostname, callback, user_data);
+
+    g_task_set_task_data(task, g_strdup(hostname), g_free);
+
+    if (priv->hostnamed_proxy) {
+        g_dbus_proxy_call(priv->hostnamed_proxy,
+                          "SetStaticHostname",
+                          g_variant_new("(sb)", hostname ?: "", FALSE),
+                          G_DBUS_CALL_FLAGS_NONE,
+                          15000,
+                          cancellable,
+                          _write_hostname_dbus_cb,
+                          task);
+        return;
+    }
+
+    nm_utils_invoke_on_idle(cancellable, _write_hostname_on_idle_cb, task);
+}
+
+gboolean
+nm_hostname_manager_write_hostname_finish(NMHostnameManager *self,
+                                          GAsyncResult      *result,
+                                          GError           **error)
+{
+    g_return_val_if_fail(NM_IS_HOSTNAME_MANAGER(self), FALSE);
+    g_return_val_if_fail(nm_g_task_is_valid(result, self, nm_hostname_manager_write_hostname),
+                         FALSE);
+
+    return g_task_propagate_boolean(G_TASK(result), error);
 }
 
 /*****************************************************************************/
diff --git a/src/core/nm-hostname-manager.h b/src/core/nm-hostname-manager.h
index b871d1779e..31596819ae 100644
--- a/src/core/nm-hostname-manager.h
+++ b/src/core/nm-hostname-manager.h
@@ -36,7 +36,15 @@ NMHostnameManager *nm_hostname_manager_get(void);
 
 const char *nm_hostname_manager_get_static_hostname(NMHostnameManager *self);
 
-gboolean nm_hostname_manager_write_hostname(NMHostnameManager *self, const char *hostname);
+void nm_hostname_manager_write_hostname(NMHostnameManager  *self,
+                                        const char         *hostname,
+                                        GCancellable       *cancellable,
+                                        GAsyncReadyCallback callback,
+                                        gpointer            user_data);
+
+gboolean nm_hostname_manager_write_hostname_finish(NMHostnameManager *self,
+                                                   GAsyncResult      *result,
+                                                   GError           **error);
 
 void nm_hostname_manager_set_transient_hostname(NMHostnameManager             *self,
                                                 const char                    *hostname,
diff --git a/src/core/settings/nm-settings.c b/src/core/settings/nm-settings.c
index 2b988527e9..b7d846c6d7 100644
--- a/src/core/settings/nm-settings.c
+++ b/src/core/settings/nm-settings.c
@@ -376,6 +376,8 @@ typedef struct {
 
     GHashTable *sce_idx;
 
+    GCancellable *shutdown_cancellable;
+
     CList sce_dirty_lst_head;
 
     CList connections_lst_head;
@@ -3466,44 +3468,93 @@ load_plugins(NMSettings *self, const char *const *plugins, GError **error)
 /*****************************************************************************/
 
 static void
-pk_hostname_cb(NMAuthChain *chain, GDBusMethodInvocation *context, gpointer user_data)
+_save_hostname_write_cb(GObject *source, GAsyncResult *result, gpointer user_data)
+{
+    NMSettings                    *self;
+    GDBusMethodInvocation         *context;
+    gs_free char                  *hostname     = NULL;
+    gs_unref_object NMAuthSubject *auth_subject = NULL;
+    gs_unref_object GCancellable  *cancellable  = NULL;
+    gs_free_error GError          *error        = NULL;
+
+    nm_utils_user_data_unpack(user_data, &self, &context, &auth_subject, &hostname, &cancellable);
+
+    nm_hostname_manager_write_hostname_finish(NM_HOSTNAME_MANAGER(source), result, &error);
+
+    nm_audit_log_control_op(NM_AUDIT_OP_HOSTNAME_SAVE,
+                            hostname ?: "",
+                            !error,
+                            auth_subject,
+                            error ? error->message : NULL);
+
+    if (nm_utils_error_is_cancelled(error)) {
+        g_dbus_method_invocation_return_error_literal(context,
+                                                      NM_SETTINGS_ERROR,
+                                                      NM_SETTINGS_ERROR_FAILED,
+                                                      "NetworkManager is shutting down");
+        return;
+    }
+
+    if (error) {
+        g_dbus_method_invocation_take_error(context,
+                                            g_error_new(NM_SETTINGS_ERROR,
+                                                        NM_SETTINGS_ERROR_FAILED,
+                                                        "Saving the hostname failed: %s",
+                                                        error->message));
+        return;
+    }
+
+    g_dbus_method_invocation_return_value(context, NULL);
+}
+
+static void
+_save_hostname_pk_cb(NMAuthChain *chain, GDBusMethodInvocation *context, gpointer user_data)
 {
     NMSettings        *self = NM_SETTINGS(user_data);
     NMSettingsPrivate *priv = NM_SETTINGS_GET_PRIVATE(self);
     NMAuthCallResult   result;
-    GError            *error = NULL;
-    const char        *hostname;
+    gs_free char      *hostname = NULL;
 
     nm_assert(G_IS_DBUS_METHOD_INVOCATION(context));
 
     c_list_unlink(nm_auth_chain_parent_lst_list(chain));
 
     result   = nm_auth_chain_get_result(chain, NM_AUTH_PERMISSION_SETTINGS_MODIFY_HOSTNAME);
-    hostname = nm_auth_chain_get_data(chain, "hostname");
+    hostname = nm_auth_chain_steal_data(chain, "hostname");
 
-    /* If our NMSettingsConnection is already gone, do nothing */
     if (result != NM_AUTH_CALL_RESULT_YES) {
-        error = g_error_new_literal(NM_SETTINGS_ERROR,
-                                    NM_SETTINGS_ERROR_PERMISSION_DENIED,
-                                    NM_UTILS_ERROR_MSG_INSUFF_PRIV);
-    } else {
-        if (!nm_hostname_manager_write_hostname(priv->hostname_manager, hostname)) {
-            error = g_error_new_literal(NM_SETTINGS_ERROR,
-                                        NM_SETTINGS_ERROR_FAILED,
-                                        "Saving the hostname failed.");
-        }
+        nm_audit_log_control_op(NM_AUDIT_OP_HOSTNAME_SAVE,
+                                hostname ?: "",
+                                FALSE,
+                                nm_auth_chain_get_subject(chain),
+                                NM_UTILS_ERROR_MSG_INSUFF_PRIV);
+        g_dbus_method_invocation_return_error_literal(context,
+                                                      NM_SETTINGS_ERROR,
+                                                      NM_SETTINGS_ERROR_PERMISSION_DENIED,
+                                                      NM_UTILS_ERROR_MSG_INSUFF_PRIV);
+        return;
     }
 
-    nm_audit_log_control_op(NM_AUDIT_OP_HOSTNAME_SAVE,
-                            hostname,
-                            !error,
-                            nm_auth_chain_get_subject(chain),
-                            error ? error->message : NULL);
+    if (!priv->shutdown_cancellable) {
+        /* we only keep a weak pointer on the cancellable, so we can
+         * wrap it up after use. We almost never require this, because
+         * SaveHostname is almost never called. */
+        priv->shutdown_cancellable = g_cancellable_new();
+        g_object_add_weak_pointer(G_OBJECT(priv->shutdown_cancellable),
+                                  (gpointer *) &priv->shutdown_cancellable);
+    }
 
-    if (error)
-        g_dbus_method_invocation_take_error(context, error);
-    else
-        g_dbus_method_invocation_return_value(context, NULL);
+    nm_hostname_manager_write_hostname(
+        priv->hostname_manager,
+        hostname,
+        priv->shutdown_cancellable,
+        _save_hostname_write_cb,
+        nm_utils_user_data_pack(self,
+                                context,
+                                g_object_ref(nm_auth_chain_get_subject(chain)),
+                                hostname,
+                                g_object_ref(priv->shutdown_cancellable)));
+    g_steal_pointer(&hostname);
 }
 
 static void
@@ -3525,13 +3576,13 @@ impl_settings_save_hostname(NMDBusObject                      *obj,
     g_variant_get(parameters, "(&s)", &hostname);
 
     /* Minimal validation of the hostname */
-    if (!nm_utils_validate_hostname(hostname)) {
+    if (nm_str_not_empty(hostname) && !nm_utils_validate_hostname(hostname)) {
         error_code   = NM_SETTINGS_ERROR_INVALID_HOSTNAME;
         error_reason = "The hostname was too long or contained invalid characters";
         goto err;
     }
 
-    chain = nm_auth_chain_new_context(invocation, pk_hostname_cb, self);
+    chain = nm_auth_chain_new_context(invocation, _save_hostname_pk_cb, self);
     if (!chain) {
         error_code   = NM_SETTINGS_ERROR_PERMISSION_DENIED;
         error_reason = NM_UTILS_ERROR_MSG_REQ_AUTH_FAILED;
@@ -3540,7 +3591,7 @@ impl_settings_save_hostname(NMDBusObject                      *obj,
 
     c_list_link_tail(&priv->auth_lst_head, nm_auth_chain_parent_lst_list(chain));
     nm_auth_chain_add_call(chain, NM_AUTH_PERMISSION_SETTINGS_MODIFY_HOSTNAME, TRUE);
-    nm_auth_chain_set_data(chain, "hostname", g_strdup(hostname), g_free);
+    nm_auth_chain_set_data(chain, "hostname", nm_strdup_not_empty(hostname), g_free);
     return;
 err:
     nm_audit_log_control_op(NM_AUDIT_OP_HOSTNAME_SAVE, hostname, FALSE, invocation, error_reason);
@@ -4124,6 +4175,12 @@ dispose(GObject *object)
         g_clear_object(&priv->session_monitor);
     }
 
+    if (priv->shutdown_cancellable) {
+        g_object_remove_weak_pointer(G_OBJECT(priv->shutdown_cancellable),
+                                     (gpointer *) &priv->shutdown_cancellable);
+        g_cancellable_cancel(g_steal_pointer(&priv->shutdown_cancellable));
+    }
+
     G_OBJECT_CLASS(nm_settings_parent_class)->dispose(object);
 }