]> git.ipfire.org Git - thirdparty/systemd.git/blobdiff - src/login/logind-dbus.c
tree-wide: Add allow_pidfd argument to bus_append_scope_pidref()
[thirdparty/systemd.git] / src / login / logind-dbus.c
index 199c7f9e9b8c754ee2b5b602088fe528855012cc..5d27491af3a5fbec9dd0265e93912a8f1427a829 100644 (file)
@@ -49,6 +49,7 @@
 #include "sleep-config.h"
 #include "special.h"
 #include "serialize.h"
+#include "signal-util.h"
 #include "stdio-util.h"
 #include "strv.h"
 #include "terminal-util.h"
@@ -70,9 +71,7 @@
 
 #define SHUTDOWN_SCHEDULE_FILE "/run/systemd/shutdown/scheduled"
 
-static int update_schedule_file(Manager *m);
 static void reset_scheduled_shutdown(Manager *m);
-static int manager_setup_shutdown_timers(Manager* m);
 
 static int get_sender_session(
                 Manager *m,
@@ -990,53 +989,41 @@ static int create_session(
                 user->gc_mode = USER_GC_BY_PIN;
 
         if (!isempty(tty)) {
-                session->tty = strdup(tty);
-                if (!session->tty) {
-                        r = -ENOMEM;
+                r = strdup_to(&session->tty, tty);
+                if (r < 0)
                         goto fail;
-                }
 
                 session->tty_validity = TTY_FROM_PAM;
         }
 
         if (!isempty(display)) {
-                session->display = strdup(display);
-                if (!session->display) {
-                        r = -ENOMEM;
+                r = strdup_to(&session->display, display);
+                if (r < 0)
                         goto fail;
-                }
         }
 
         if (!isempty(remote_user)) {
-                session->remote_user = strdup(remote_user);
-                if (!session->remote_user) {
-                        r = -ENOMEM;
+                r = strdup_to(&session->remote_user, remote_user);
+                if (r < 0)
                         goto fail;
-                }
         }
 
         if (!isempty(remote_host)) {
-                session->remote_host = strdup(remote_host);
-                if (!session->remote_host) {
-                        r = -ENOMEM;
+                r = strdup_to(&session->remote_host, remote_host);
+                if (r < 0)
                         goto fail;
-                }
         }
 
         if (!isempty(service)) {
-                session->service = strdup(service);
-                if (!session->service) {
-                        r = -ENOMEM;
+                r = strdup_to(&session->service, service);
+                if (r < 0)
                         goto fail;
-                }
         }
 
         if (!isempty(desktop)) {
-                session->desktop = strdup(desktop);
-                if (!session->desktop) {
-                        r = -ENOMEM;
+                r = strdup_to(&session->desktop, desktop);
+                if (r < 0)
                         goto fail;
-                }
         }
 
         if (seat) {
@@ -1443,8 +1430,8 @@ static int method_set_user_linger(sd_bus_message *message, void *userdata, sd_bu
                         uid == auth_uid ? "org.freedesktop.login1.set-self-linger" :
                                           "org.freedesktop.login1.set-user-linger",
                         /* details= */ NULL,
-                        interactive,
                         /* good_user= */ UID_INVALID,
+                        interactive ? POLKIT_ALLOW_INTERACTIVE : 0,
                         &m->polkit_registry,
                         error);
         if (r < 0)
@@ -1615,8 +1602,8 @@ static int method_attach_device(sd_bus_message *message, void *userdata, sd_bus_
                         message,
                         "org.freedesktop.login1.attach-device",
                         /* details= */ NULL,
-                        interactive,
                         /* good_user= */ UID_INVALID,
+                        interactive ? POLKIT_ALLOW_INTERACTIVE : 0,
                         &m->polkit_registry,
                         error);
         if (r < 0)
@@ -1645,8 +1632,8 @@ static int method_flush_devices(sd_bus_message *message, void *userdata, sd_bus_
                         message,
                         "org.freedesktop.login1.flush-devices",
                         /* details= */ NULL,
-                        interactive,
                         /* good_user= */ UID_INVALID,
+                        interactive ? POLKIT_ALLOW_INTERACTIVE : 0,
                         &m->polkit_registry,
                         error);
         if (r < 0)
@@ -1672,7 +1659,7 @@ static int have_multiple_sessions(
         /* Check for other users' sessions. Greeter sessions do not
          * count, and non-login sessions do not count either. */
         HASHMAP_FOREACH(session, m->sessions)
-                if (session->class == SESSION_USER &&
+                if (IN_SET(session->class, SESSION_USER, SESSION_USER_EARLY) &&
                     session->user->user_record->uid != uid)
                         return true;
 
@@ -1781,13 +1768,32 @@ static int send_prepare_for(Manager *m, const HandleActionData *a, bool _active)
         return RET_GATHER(k, r);
 }
 
+static int strdup_job(sd_bus_message *reply, char **ret) {
+        const char *j;
+        char *job;
+        int r;
+
+        assert(reply);
+        assert(ret);
+
+        r = sd_bus_message_read_basic(reply, 'o', &j);
+        if (r < 0)
+                return r;
+
+        job = strdup(j);
+        if (!job)
+                return -ENOMEM;
+
+        *ret = job;
+        return 0;
+}
+
 static int execute_shutdown_or_sleep(
                 Manager *m,
                 const HandleActionData *a,
                 sd_bus_error *error) {
 
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
-        const char *p;
         int r;
 
         assert(m);
@@ -1805,17 +1811,11 @@ static int execute_shutdown_or_sleep(
                         &reply,
                         "ss", a->target, "replace-irreversibly");
         if (r < 0)
-                goto error;
+                goto fail;
 
-        r = sd_bus_message_read(reply, "o", &p);
+        r = strdup_job(reply, &m->action_job);
         if (r < 0)
-                goto error;
-
-        m->action_job = strdup(p);
-        if (!m->action_job) {
-                r = -ENOMEM;
-                goto error;
-        }
+                goto fail;
 
         m->delayed_action = a;
 
@@ -1824,7 +1824,7 @@ static int execute_shutdown_or_sleep(
 
         return 0;
 
-error:
+fail:
         /* Tell people that they now may take a lock again */
         (void) send_prepare_for(m, a, false);
 
@@ -1989,8 +1989,8 @@ static int verify_shutdown_creds(
                                 message,
                                 a->polkit_action_multiple_sessions,
                                 /* details= */ NULL,
-                                interactive,
                                 /* good_user= */ UID_INVALID,
+                                interactive ? POLKIT_ALLOW_INTERACTIVE : 0,
                                 &m->polkit_registry,
                                 error);
                 if (r < 0)
@@ -2009,8 +2009,8 @@ static int verify_shutdown_creds(
                                 message,
                                 a->polkit_action_ignore_inhibit,
                                 /* details= */ NULL,
-                                interactive,
                                 /* good_user= */ UID_INVALID,
+                                interactive ? POLKIT_ALLOW_INTERACTIVE : 0,
                                 &m->polkit_registry,
                                 error);
                 if (r < 0)
@@ -2024,8 +2024,8 @@ static int verify_shutdown_creds(
                                 message,
                                 a->polkit_action,
                                 /* details= */ NULL,
-                                interactive,
                                 /* good_user= */ UID_INVALID,
+                                interactive ? POLKIT_ALLOW_INTERACTIVE : 0,
                                 &m->polkit_registry,
                                 error);
                 if (r < 0)
@@ -2278,7 +2278,7 @@ static int nologin_timeout_handler(
                         uint64_t usec,
                         void *userdata) {
 
-        Manager *m = userdata;
+        Manager *m = ASSERT_PTR(userdata);
 
         log_info("Creating /run/nologin, blocking further logins...");
 
@@ -2293,79 +2293,25 @@ static usec_t nologin_timeout_usec(usec_t elapse) {
         return LESS_BY(elapse, 5 * USEC_PER_MINUTE);
 }
 
-void manager_load_scheduled_shutdown(Manager *m) {
-        _cleanup_fclose_ FILE *f = NULL;
-        _cleanup_free_ char *usec = NULL,
-               *warn_wall = NULL,
-               *mode = NULL,
-               *wall_message = NULL,
-               *uid = NULL,
-               *tty = NULL;
-        int r;
-
+static void reset_scheduled_shutdown(Manager *m) {
         assert(m);
 
-        r = parse_env_file(f, SHUTDOWN_SCHEDULE_FILE,
-                        "USEC", &usec,
-                        "WARN_WALL", &warn_wall,
-                        "MODE", &mode,
-                        "WALL_MESSAGE", &wall_message,
-                        "UID", &uid,
-                        "TTY", &tty);
-
-        /* reset will delete the file */
-        reset_scheduled_shutdown(m);
-
-        if (r == -ENOENT)
-                return;
-        if (r < 0)
-                return (void) log_debug_errno(r, "Failed to parse " SHUTDOWN_SCHEDULE_FILE ": %m");
-
-        HandleAction handle = handle_action_from_string(mode);
-        if (handle < 0)
-                return (void) log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to parse scheduled shutdown type: %s", mode);
-
-        if (!usec)
-                return (void) log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "USEC is required");
-        if (deserialize_usec(usec, &m->scheduled_shutdown_timeout) < 0)
-                return;
-
-        /* assign parsed type only after we know usec is also valid */
-        m->scheduled_shutdown_action = handle;
-
-        if (warn_wall) {
-                r = parse_boolean(warn_wall);
-                if (r < 0)
-                        log_debug_errno(r, "Failed to parse enabling wall messages");
-                else
-                        m->enable_wall_messages = r;
-        }
+        m->scheduled_shutdown_timeout_source = sd_event_source_unref(m->scheduled_shutdown_timeout_source);
+        m->wall_message_timeout_source = sd_event_source_unref(m->wall_message_timeout_source);
+        m->nologin_timeout_source = sd_event_source_unref(m->nologin_timeout_source);
 
-        if (wall_message) {
-                _cleanup_free_ char *unescaped = NULL;
-                r = cunescape(wall_message, 0, &unescaped);
-                if (r < 0)
-                        log_debug_errno(r, "Failed to parse wall message: %s", wall_message);
-                else
-                        free_and_replace(m->wall_message, unescaped);
-        }
+        m->scheduled_shutdown_action = _HANDLE_ACTION_INVALID;
+        m->scheduled_shutdown_timeout = USEC_INFINITY;
+        m->scheduled_shutdown_uid = UID_INVALID;
+        m->scheduled_shutdown_tty = mfree(m->scheduled_shutdown_tty);
+        m->shutdown_dry_run = false;
 
-        if (uid) {
-                r = parse_uid(uid, &m->scheduled_shutdown_uid);
-                if (r < 0)
-                        log_debug_errno(r, "Failed to parse wall uid: %s", uid);
+        if (m->unlink_nologin) {
+                (void) unlink_or_warn("/run/nologin");
+                m->unlink_nologin = false;
         }
 
-        free_and_replace(m->scheduled_shutdown_tty, tty);
-
-        r = manager_setup_shutdown_timers(m);
-        if (r < 0)
-                return reset_scheduled_shutdown(m);
-
-        (void) manager_setup_wall_message_timer(m);
-        (void) update_schedule_file(m);
-
-        return;
+        (void) unlink(SHUTDOWN_SCHEDULE_FILE);
 }
 
 static int update_schedule_file(Manager *m) {
@@ -2418,27 +2364,6 @@ fail:
         return log_error_errno(r, "Failed to write information about scheduled shutdowns: %m");
 }
 
-static void reset_scheduled_shutdown(Manager *m) {
-        assert(m);
-
-        m->scheduled_shutdown_timeout_source = sd_event_source_unref(m->scheduled_shutdown_timeout_source);
-        m->wall_message_timeout_source = sd_event_source_unref(m->wall_message_timeout_source);
-        m->nologin_timeout_source = sd_event_source_unref(m->nologin_timeout_source);
-
-        m->scheduled_shutdown_action = _HANDLE_ACTION_INVALID;
-        m->scheduled_shutdown_timeout = USEC_INFINITY;
-        m->scheduled_shutdown_uid = UID_INVALID;
-        m->scheduled_shutdown_tty = mfree(m->scheduled_shutdown_tty);
-        m->shutdown_dry_run = false;
-
-        if (m->unlink_nologin) {
-                (void) unlink_or_warn("/run/nologin");
-                m->unlink_nologin = false;
-        }
-
-        (void) unlink(SHUTDOWN_SCHEDULE_FILE);
-}
-
 static int manager_scheduled_shutdown_handler(
                         sd_event_source *s,
                         uint64_t usec,
@@ -2485,6 +2410,111 @@ error:
         return r;
 }
 
+static int manager_setup_shutdown_timers(Manager* m) {
+        int r;
+
+        assert(m);
+
+        r = event_reset_time(m->event, &m->scheduled_shutdown_timeout_source,
+                             CLOCK_REALTIME,
+                             m->scheduled_shutdown_timeout, 0,
+                             manager_scheduled_shutdown_handler, m,
+                             0, "scheduled-shutdown-timeout", true);
+        if (r < 0)
+                goto fail;
+
+        r = event_reset_time(m->event, &m->nologin_timeout_source,
+                             CLOCK_REALTIME,
+                             nologin_timeout_usec(m->scheduled_shutdown_timeout), 0,
+                             nologin_timeout_handler, m,
+                             0, "nologin-timeout", true);
+        if (r < 0)
+                goto fail;
+
+        return 0;
+
+fail:
+        m->scheduled_shutdown_timeout_source = sd_event_source_unref(m->scheduled_shutdown_timeout_source);
+        m->nologin_timeout_source = sd_event_source_unref(m->nologin_timeout_source);
+
+        return r;
+}
+
+void manager_load_scheduled_shutdown(Manager *m) {
+        _cleanup_fclose_ FILE *f = NULL;
+        _cleanup_free_ char *usec = NULL,
+               *warn_wall = NULL,
+               *mode = NULL,
+               *wall_message = NULL,
+               *uid = NULL,
+               *tty = NULL;
+        int r;
+
+        assert(m);
+
+        r = parse_env_file(f, SHUTDOWN_SCHEDULE_FILE,
+                           "USEC", &usec,
+                           "WARN_WALL", &warn_wall,
+                           "MODE", &mode,
+                           "WALL_MESSAGE", &wall_message,
+                           "UID", &uid,
+                           "TTY", &tty);
+
+        /* reset will delete the file */
+        reset_scheduled_shutdown(m);
+
+        if (r == -ENOENT)
+                return;
+        if (r < 0)
+                return (void) log_debug_errno(r, "Failed to parse " SHUTDOWN_SCHEDULE_FILE ": %m");
+
+        HandleAction handle = handle_action_from_string(mode);
+        if (handle < 0)
+                return (void) log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "Failed to parse scheduled shutdown type: %s", mode);
+
+        if (!usec)
+                return (void) log_debug_errno(SYNTHETIC_ERRNO(EINVAL), "USEC is required");
+        if (deserialize_usec(usec, &m->scheduled_shutdown_timeout) < 0)
+                return;
+
+        /* assign parsed type only after we know usec is also valid */
+        m->scheduled_shutdown_action = handle;
+
+        if (warn_wall) {
+                r = parse_boolean(warn_wall);
+                if (r < 0)
+                        log_debug_errno(r, "Failed to parse enabling wall messages");
+                else
+                        m->enable_wall_messages = r;
+        }
+
+        if (wall_message) {
+                _cleanup_free_ char *unescaped = NULL;
+                r = cunescape(wall_message, 0, &unescaped);
+                if (r < 0)
+                        log_debug_errno(r, "Failed to parse wall message: %s", wall_message);
+                else
+                        free_and_replace(m->wall_message, unescaped);
+        }
+
+        if (uid) {
+                r = parse_uid(uid, &m->scheduled_shutdown_uid);
+                if (r < 0)
+                        log_debug_errno(r, "Failed to parse wall uid: %s", uid);
+        }
+
+        free_and_replace(m->scheduled_shutdown_tty, tty);
+
+        r = manager_setup_shutdown_timers(m);
+        if (r < 0)
+                return reset_scheduled_shutdown(m);
+
+        (void) manager_setup_wall_message_timer(m);
+        (void) update_schedule_file(m);
+
+        return;
+}
+
 static int method_schedule_shutdown(sd_bus_message *message, void *userdata, sd_bus_error *error) {
         Manager *m = ASSERT_PTR(userdata);
         HandleAction handle;
@@ -2536,34 +2566,6 @@ static int method_schedule_shutdown(sd_bus_message *message, void *userdata, sd_
         return sd_bus_reply_method_return(message, NULL);
 }
 
-static int manager_setup_shutdown_timers(Manager* m) {
-        int r;
-
-        r = event_reset_time(m->event, &m->scheduled_shutdown_timeout_source,
-                             CLOCK_REALTIME,
-                             m->scheduled_shutdown_timeout, 0,
-                             manager_scheduled_shutdown_handler, m,
-                             0, "scheduled-shutdown-timeout", true);
-        if (r < 0)
-                goto fail;
-
-        r = event_reset_time(m->event, &m->nologin_timeout_source,
-                             CLOCK_REALTIME,
-                             nologin_timeout_usec(m->scheduled_shutdown_timeout), 0,
-                             nologin_timeout_handler, m,
-                             0, "nologin-timeout", true);
-        if (r < 0)
-                goto fail;
-
-        return 0;
-
-fail:
-        m->scheduled_shutdown_timeout_source = sd_event_source_unref(m->scheduled_shutdown_timeout_source);
-        m->nologin_timeout_source = sd_event_source_unref(m->nologin_timeout_source);
-
-        return r;
-}
-
 static int method_cancel_scheduled_shutdown(sd_bus_message *message, void *userdata, sd_bus_error *error) {
         Manager *m = ASSERT_PTR(userdata);
         const HandleActionData *a;
@@ -4038,22 +4040,26 @@ const BusObjectImplementation manager_object = {
                                         &user_object),
 };
 
-static int session_jobs_reply(Session *s, uint32_t jid, const char *unit, const char *result) {
+static void session_jobs_reply(Session *s, uint32_t jid, const char *unit, const char *result) {
         assert(s);
         assert(unit);
 
         if (!s->started)
-                return 0;
+                return;
 
         if (result && !streq(result, "done")) {
                 _cleanup_(sd_bus_error_free) sd_bus_error e = SD_BUS_ERROR_NULL;
 
                 sd_bus_error_setf(&e, BUS_ERROR_JOB_FAILED,
                                   "Job %u for unit '%s' failed with '%s'", jid, unit, result);
-                return session_send_create_reply(s, &e);
+
+                (void) session_send_create_reply(s, &e);
+                (void) session_send_upgrade_reply(s, &e);
+                return;
         }
 
-        return session_send_create_reply(s, NULL);
+        (void) session_send_create_reply(s, /* error= */ NULL);
+        (void) session_send_upgrade_reply(s, /* error= */ NULL);
 }
 
 int match_job_removed(sd_bus_message *message, void *userdata, sd_bus_error *error) {
@@ -4089,7 +4095,7 @@ int match_job_removed(sd_bus_message *message, void *userdata, sd_bus_error *err
         if (session) {
                 if (streq_ptr(path, session->scope_job)) {
                         session->scope_job = mfree(session->scope_job);
-                        (void) session_jobs_reply(session, id, unit, result);
+                        session_jobs_reply(session, id, unit, result);
 
                         session_save(session);
                         user_save(session->user);
@@ -4101,14 +4107,19 @@ int match_job_removed(sd_bus_message *message, void *userdata, sd_bus_error *err
         user = hashmap_get(m->user_units, unit);
         if (user) {
                 /* If the user is stopping, we're tracking stop jobs here. So don't send reply. */
-                if (!user->stopping && streq_ptr(path, user->service_job)) {
-                        user->service_job = mfree(user->service_job);
-
-                        LIST_FOREACH(sessions_by_user, s, user->sessions)
-                                /* Don't propagate user service failures to the client */
-                                (void) session_jobs_reply(s, id, unit, /* error = */ NULL);
-
-                        user_save(user);
+                if (!user->stopping) {
+                        char **user_job;
+                        FOREACH_ARGUMENT(user_job, &user->runtime_dir_job, &user->service_manager_job)
+                                if (streq_ptr(path, *user_job)) {
+                                        *user_job = mfree(*user_job);
+
+                                        LIST_FOREACH(sessions_by_user, s, user->sessions)
+                                                /* Don't propagate user service failures to the client */
+                                                session_jobs_reply(s, id, unit, /* error = */ NULL);
+
+                                        user_save(user);
+                                        break;
+                                }
                 }
 
                 user_add_to_gc_queue(user);
@@ -4215,35 +4226,19 @@ int manager_send_changed(Manager *manager, const char *property, ...) {
                         l);
 }
 
-static int strdup_job(sd_bus_message *reply, char **job) {
-        const char *j;
-        char *copy;
-        int r;
-
-        r = sd_bus_message_read(reply, "o", &j);
-        if (r < 0)
-                return r;
-
-        copy = strdup(j);
-        if (!copy)
-                return -ENOMEM;
-
-        *job = copy;
-        return 1;
-}
-
 int manager_start_scope(
                 Manager *manager,
                 const char *scope,
                 const PidRef *pidref,
+                bool allow_pidfd,
                 const char *slice,
                 const char *description,
-                char **wants,
-                char **after,
+                const char * const *requires,
+                const char * const *extra_after,
                 const char *requires_mounts_for,
                 sd_bus_message *more_properties,
                 sd_bus_error *error,
-                char **job) {
+                char **ret_job) {
 
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL, *reply = NULL;
         int r;
@@ -4251,13 +4246,13 @@ int manager_start_scope(
         assert(manager);
         assert(scope);
         assert(pidref_is_set(pidref));
-        assert(job);
+        assert(ret_job);
 
         r = bus_message_new_method_call(manager->bus, &m, bus_systemd_mgr, "StartTransientUnit");
         if (r < 0)
                 return r;
 
-        r = sd_bus_message_append(m, "ss", strempty(scope), "fail");
+        r = sd_bus_message_append(m, "ss", scope, "fail");
         if (r < 0)
                 return r;
 
@@ -4277,13 +4272,17 @@ int manager_start_scope(
                         return r;
         }
 
-        STRV_FOREACH(i, wants) {
-                r = sd_bus_message_append(m, "(sv)", "Wants", "as", 1, *i);
+        STRV_FOREACH(i, requires) {
+                r = sd_bus_message_append(m, "(sv)", "Requires", "as", 1, *i);
+                if (r < 0)
+                        return r;
+
+                r = sd_bus_message_append(m, "(sv)", "After", "as", 1, *i);
                 if (r < 0)
                         return r;
         }
 
-        STRV_FOREACH(i, after) {
+        STRV_FOREACH(i, extra_after) {
                 r = sd_bus_message_append(m, "(sv)", "After", "as", 1, *i);
                 if (r < 0)
                         return r;
@@ -4301,7 +4300,7 @@ int manager_start_scope(
         if (r < 0)
                 return r;
 
-        r = bus_append_scope_pidref(m, pidref);
+        r = bus_append_scope_pidref(m, pidref, allow_pidfd);
         if (r < 0)
                 return r;
 
@@ -4332,19 +4331,38 @@ int manager_start_scope(
                 return r;
 
         r = sd_bus_call(manager->bus, m, 0, error, &reply);
-        if (r < 0)
+        if (r < 0) {
+                /* If this failed with a property we couldn't write, this is quite likely because the server
+                 * doesn't support PIDFDs yet, let's try without. */
+                if (allow_pidfd &&
+                    sd_bus_error_has_names(error, SD_BUS_ERROR_UNKNOWN_PROPERTY, SD_BUS_ERROR_PROPERTY_READ_ONLY))
+                        return manager_start_scope(
+                                        manager,
+                                        scope,
+                                        pidref,
+                                        /* allow_pidfd = */ false,
+                                        slice,
+                                        description,
+                                        requires,
+                                        extra_after,
+                                        requires_mounts_for,
+                                        more_properties,
+                                        error,
+                                        ret_job);
+
                 return r;
+        }
 
-        return strdup_job(reply, job);
+        return strdup_job(reply, ret_job);
 }
 
-int manager_start_unit(Manager *manager, const char *unit, sd_bus_error *error, char **job) {
+int manager_start_unit(Manager *manager, const char *unit, sd_bus_error *error, char **ret_job) {
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
         int r;
 
         assert(manager);
         assert(unit);
-        assert(job);
+        assert(ret_job);
 
         r = bus_call_method(
                         manager->bus,
@@ -4356,11 +4374,18 @@ int manager_start_unit(Manager *manager, const char *unit, sd_bus_error *error,
         if (r < 0)
                 return r;
 
-        return strdup_job(reply, job);
+        return strdup_job(reply, ret_job);
 }
 
-int manager_stop_unit(Manager *manager, const char *unit, const char *job_mode, sd_bus_error *error, char **ret_job) {
+int manager_stop_unit(
+                Manager *manager,
+                const char *unit,
+                const char *job_mode,
+                sd_bus_error *ret_error,
+                char **ret_job) {
+
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
+        _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
         int r;
 
         assert(manager);
@@ -4371,22 +4396,24 @@ int manager_stop_unit(Manager *manager, const char *unit, const char *job_mode,
                         manager->bus,
                         bus_systemd_mgr,
                         "StopUnit",
-                        error,
+                        &error,
                         &reply,
                         "ss", unit, job_mode ?: "fail");
         if (r < 0) {
-                if (sd_bus_error_has_names(error, BUS_ERROR_NO_SUCH_UNIT,
-                                                  BUS_ERROR_LOAD_FAILED)) {
-
+                if (sd_bus_error_has_names(&error, BUS_ERROR_NO_SUCH_UNIT, BUS_ERROR_LOAD_FAILED)) {
                         *ret_job = NULL;
-                        sd_bus_error_free(error);
                         return 0;
                 }
 
+                sd_bus_error_move(ret_error, &error);
                 return r;
         }
 
-        return strdup_job(reply, ret_job);
+        r = strdup_job(reply, ret_job);
+        if (r < 0)
+                return r;
+
+        return 1;
 }
 
 int manager_abandon_scope(Manager *manager, const char *scope, sd_bus_error *ret_error) {
@@ -4426,6 +4453,7 @@ int manager_abandon_scope(Manager *manager, const char *scope, sd_bus_error *ret
 int manager_kill_unit(Manager *manager, const char *unit, KillWho who, int signo, sd_bus_error *error) {
         assert(manager);
         assert(unit);
+        assert(SIGNAL_VALID(signo));
 
         return bus_call_method(
                         manager->bus,
@@ -4433,7 +4461,10 @@ int manager_kill_unit(Manager *manager, const char *unit, KillWho who, int signo
                         "KillUnit",
                         error,
                         NULL,
-                        "ssi", unit, who == KILL_LEADER ? "main" : "all", signo);
+                        "ssi",
+                        unit,
+                        who == KILL_LEADER ? "main" : "all",
+                        signo);
 }
 
 int manager_unit_is_active(Manager *manager, const char *unit, sd_bus_error *ret_error) {