diff --git a/src/login/inhibit.c b/src/login/inhibit.c index d96c8195a2..25ff4d0be6 100644 --- a/src/login/inhibit.c +++ b/src/login/inhibit.c @@ -284,7 +284,6 @@ static int run(int argc, char *argv[]) { if (arg_action == ACTION_LIST) return print_inhibitors(bus); - else { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_strv_free_ char **arguments = NULL; @@ -295,8 +294,13 @@ static int run(int argc, char *argv[]) { /* Ignore SIGINT and allow the forked process to receive it */ (void) ignore_signals(SIGINT, -1); - if (!arg_who) - arg_who = w = strv_join(argv + optind, " "); + if (!arg_who) { + w = strv_join(argv + optind, " "); + if (!w) + return log_oom(); + + arg_who = w; + } if (!arg_mode) arg_mode = "block"; diff --git a/src/login/logind-core.c b/src/login/logind-core.c index f631ba7219..1d21e90a2e 100644 --- a/src/login/logind-core.c +++ b/src/login/logind-core.c @@ -197,26 +197,22 @@ int manager_add_user_by_uid(Manager *m, uid_t uid, User **_user) { return manager_add_user(m, uid, p->pw_gid, p->pw_name, p->pw_dir, _user); } -int manager_add_inhibitor(Manager *m, const char* id, Inhibitor **_inhibitor) { +int manager_add_inhibitor(Manager *m, const char* id, Inhibitor **ret) { Inhibitor *i; + int r; assert(m); assert(id); i = hashmap_get(m->inhibitors, id); - if (i) { - if (_inhibitor) - *_inhibitor = i; - - return 0; + if (!i) { + r = inhibitor_new(&i, m, id); + if (r < 0) + return r; } - i = inhibitor_new(m, id); - if (!i) - return -ENOMEM; - - if (_inhibitor) - *_inhibitor = i; + if (ret) + *ret = i; return 0; } diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index b7c6e00e95..b36616e55a 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -3288,7 +3288,9 @@ static int method_inhibit(sd_bus_message *message, void *userdata, sd_bus_error goto fail; } - inhibitor_start(i); + r = inhibitor_start(i); + if (r < 0) + goto fail; return sd_bus_reply_method_return(message, "h", fifo_fd); diff --git a/src/login/logind-inhibit.c b/src/login/logind-inhibit.c index d32e299101..8a36155115 100644 --- a/src/login/logind-inhibit.c +++ b/src/login/logind-inhibit.c @@ -13,6 +13,7 @@ #include "fd-util.h" #include "fileio.h" #include "format-util.h" +#include "io-util.h" #include "logind-dbus.h" #include "logind-inhibit.h" #include "mkdir.h" @@ -24,51 +25,64 @@ #include "user-util.h" #include "util.h" -Inhibitor* inhibitor_new(Manager *m, const char* id) { - Inhibitor *i; +static void inhibitor_remove_fifo(Inhibitor *i); +int inhibitor_new(Inhibitor **ret, Manager *m, const char* id) { + _cleanup_(inhibitor_freep) Inhibitor *i = NULL; + int r; + + assert(ret); assert(m); + assert(id); - i = new0(Inhibitor, 1); + i = new(Inhibitor, 1); if (!i) - return NULL; + return -ENOMEM; + + *i = (Inhibitor) { + .manager = m, + .what = _INHIBIT_WHAT_INVALID, + .mode = _INHIBIT_MODE_INVALID, + .uid = UID_INVALID, + .fifo_fd = -1, + }; i->state_file = path_join("/run/systemd/inhibit", id); if (!i->state_file) - return mfree(i); + return -ENOMEM; i->id = basename(i->state_file); - if (hashmap_put(m->inhibitors, i->id, i) < 0) { - free(i->state_file); - return mfree(i); - } + r = hashmap_put(m->inhibitors, i->id, i); + if (r < 0) + return r; - i->manager = m; - i->fifo_fd = -1; - - return i; + *ret = TAKE_PTR(i); + return 0; } -void inhibitor_free(Inhibitor *i) { - assert(i); +Inhibitor* inhibitor_free(Inhibitor *i) { - hashmap_remove(i->manager->inhibitors, i->id); - - inhibitor_remove_fifo(i); + if (!i) + return NULL; free(i->who); free(i->why); - if (i->state_file) { - (void) unlink(i->state_file); - free(i->state_file); - } + sd_event_source_unref(i->event_source); + safe_close(i->fifo_fd); - free(i); + /* Note that we don't remove neither the state file nor the fifo path here, since we want both to + * survive daemon restarts */ + free(i->state_file); + free(i->fifo_path); + + hashmap_remove(i->manager->inhibitors, i->id); + + return mfree(i); } -int inhibitor_save(Inhibitor *i) { +static int inhibitor_save(Inhibitor *i) { _cleanup_free_ char *temp_path = NULL; _cleanup_fclose_ FILE *f = NULL; int r; @@ -83,7 +97,7 @@ int inhibitor_save(Inhibitor *i) { if (r < 0) goto fail; - fchmod(fileno(f), 0644); + (void) fchmod(fileno(f), 0644); fprintf(f, "# This is private data. Do not parse.\n" @@ -143,6 +157,16 @@ fail: return log_error_errno(r, "Failed to save inhibit data %s: %m", i->state_file); } +static int bus_manager_send_inhibited_change(Inhibitor *i) { + const char *property; + + assert(i); + + property = i->mode == INHIBIT_BLOCK ? "BlockInhibited" : "DelayInhibited"; + + return manager_send_changed(i->manager, property, NULL); +} + int inhibitor_start(Inhibitor *i) { assert(i); @@ -156,16 +180,16 @@ int inhibitor_start(Inhibitor *i) { i->pid, i->uid, inhibit_mode_to_string(i->mode)); - inhibitor_save(i); - i->started = true; - manager_send_changed(i->manager, i->mode == INHIBIT_BLOCK ? "BlockInhibited" : "DelayInhibited", NULL); + inhibitor_save(i); + + bus_manager_send_inhibited_change(i); return 0; } -int inhibitor_stop(Inhibitor *i) { +void inhibitor_stop(Inhibitor *i) { assert(i); if (i->started) @@ -174,14 +198,14 @@ int inhibitor_stop(Inhibitor *i) { i->pid, i->uid, inhibit_mode_to_string(i->mode)); + inhibitor_remove_fifo(i); + if (i->state_file) (void) unlink(i->state_file); i->started = false; - manager_send_changed(i->manager, i->mode == INHIBIT_BLOCK ? "BlockInhibited" : "DelayInhibited", NULL); - - return 0; + bus_manager_send_inhibited_change(i); } int inhibitor_load(Inhibitor *i) { @@ -208,7 +232,7 @@ int inhibitor_load(Inhibitor *i) { "MODE", &mode, "FIFO", &i->fifo_path); if (r < 0) - return r; + return log_error_errno(r, "Failed to read %s: %m", i->state_file); w = what ? inhibit_what_from_string(what) : 0; if (w >= 0) @@ -221,38 +245,38 @@ int inhibitor_load(Inhibitor *i) { if (uid) { r = parse_uid(uid, &i->uid); if (r < 0) - return r; + log_debug_errno(r, "Failed to parse UID of inhibitor: %s", uid); } if (pid) { r = parse_pid(pid, &i->pid); if (r < 0) - return r; + log_debug_errno(r, "Failed to parse PID of inhibitor: %s", pid); } if (who) { r = cunescape(who, 0, &cc); if (r < 0) - return r; + return log_oom(); - free(i->who); - i->who = cc; + free_and_replace(i->who, cc); } if (why) { r = cunescape(why, 0, &cc); if (r < 0) - return r; + return log_oom(); - free(i->why); - i->why = cc; + free_and_replace(i->why, cc); } if (i->fifo_path) { - int fd; + _cleanup_close_ int fd = -1; + /* Let's re-open the FIFO on both sides, and close the writing side right away */ fd = inhibitor_create_fifo(i); - safe_close(fd); + if (fd < 0) + return log_error_errno(fd, "Failed to reopen FIFO: %m"); } return 0; @@ -315,7 +339,7 @@ int inhibitor_create_fifo(Inhibitor *i) { return r; } -void inhibitor_remove_fifo(Inhibitor *i) { +static void inhibitor_remove_fifo(Inhibitor *i) { assert(i); i->event_source = sd_event_source_unref(i->event_source); @@ -327,6 +351,24 @@ void inhibitor_remove_fifo(Inhibitor *i) { } } +bool inhibitor_is_orphan(Inhibitor *i) { + assert(i); + + if (!i->started) + return true; + + if (!i->fifo_path) + return true; + + if (i->fifo_fd < 0) + return true; + + if (pipe_eof(i->fifo_fd) != 0) + return true; + + return false; +} + InhibitWhat manager_inhibit_what(Manager *m, InhibitMode mm) { Inhibitor *i; Iterator j; diff --git a/src/login/logind-inhibit.h b/src/login/logind-inhibit.h index 650587106d..cea67a08c5 100644 --- a/src/login/logind-inhibit.h +++ b/src/login/logind-inhibit.h @@ -29,7 +29,7 @@ struct Inhibitor { sd_event_source *event_source; - char *id; + const char *id; char *state_file; bool started; @@ -48,17 +48,19 @@ struct Inhibitor { int fifo_fd; }; -Inhibitor* inhibitor_new(Manager *m, const char *id); -void inhibitor_free(Inhibitor *i); +int inhibitor_new(Inhibitor **ret, Manager *m, const char* id); +Inhibitor* inhibitor_free(Inhibitor *i); + +DEFINE_TRIVIAL_CLEANUP_FUNC(Inhibitor*, inhibitor_free); -int inhibitor_save(Inhibitor *i); int inhibitor_load(Inhibitor *i); int inhibitor_start(Inhibitor *i); -int inhibitor_stop(Inhibitor *i); +void inhibitor_stop(Inhibitor *i); int inhibitor_create_fifo(Inhibitor *i); -void inhibitor_remove_fifo(Inhibitor *i); + +bool inhibitor_is_orphan(Inhibitor *i); InhibitWhat manager_inhibit_what(Manager *m, InhibitMode mm); bool manager_is_inhibited(Manager *m, InhibitWhat w, InhibitMode mm, dual_timestamp *since, bool ignore_inactive, bool ignore_uid, uid_t uid, Inhibitor **offending); diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c index 9577ff86f4..094fcd668d 100644 --- a/src/login/logind-seat.c +++ b/src/login/logind-seat.c @@ -191,14 +191,14 @@ int seat_preallocate_vts(Seat *s) { assert(s); assert(s->manager); - log_debug("Preallocating VTs..."); - if (s->manager->n_autovts <= 0) return 0; if (!seat_has_vts(s)) return 0; + log_debug("Preallocating VTs..."); + for (i = 1; i <= s->manager->n_autovts; i++) { int q; diff --git a/src/login/logind-session.c b/src/login/logind-session.c index f78453736d..7e8025a0ea 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -96,8 +96,6 @@ Session* session_free(Session *s) { s->timer_event_source = sd_event_source_unref(s->timer_event_source); - session_remove_fifo(s); - session_drop_controller(s); while ((sd = hashmap_first(s->devices))) @@ -145,7 +143,13 @@ Session* session_free(Session *s) { hashmap_remove(s->manager->sessions, s->id); + sd_event_source_unref(s->fifo_event_source); + safe_close(s->fifo_fd); + + /* Note that we remove neither the state file nor the fifo path here, since we want both to survive + * daemon restarts */ free(s->state_file); + free(s->fifo_path); return mfree(s); } @@ -1181,7 +1185,7 @@ static int session_open_vt(Session *s) { int session_prepare_vt(Session *s) { int vt, r; - struct vt_mode mode = { 0 }; + struct vt_mode mode = {}; if (s->vtnr < 1) return 0; diff --git a/src/login/logind.c b/src/login/logind.c index f4bc1d2eb7..3a16e6c871 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -295,10 +295,8 @@ static int manager_enumerate_linger_users(Manager *m) { continue; k = manager_add_user_by_name(m, de->d_name, NULL); - if (k < 0) { - log_notice_errno(k, "Couldn't add lingering user %s: %m", de->d_name); - r = k; - } + if (k < 0) + r = log_warning_errno(k, "Couldn't add lingering user %s, ignoring: %m", de->d_name); } return r; @@ -331,9 +329,7 @@ static int manager_enumerate_users(Manager *m) { k = manager_add_user_by_name(m, de->d_name, &u); if (k < 0) { - log_error_errno(k, "Failed to add user by file name %s: %m", de->d_name); - - r = k; + r = log_warning_errno(k, "Failed to add user by file name %s, ignoring: %m", de->d_name); continue; } @@ -478,16 +474,9 @@ static int manager_enumerate_sessions(Manager *m) { if (!dirent_is_file(de)) continue; - if (!session_id_valid(de->d_name)) { - log_warning("Invalid session file name '%s', ignoring.", de->d_name); - r = -EINVAL; - continue; - } - k = manager_add_session(m, de->d_name, &s); if (k < 0) { - log_error_errno(k, "Failed to add session by file name %s: %m", de->d_name); - r = k; + r = log_warning_errno(k, "Failed to add session by file name %s, ignoring: %m", de->d_name); continue; } @@ -529,8 +518,7 @@ static int manager_enumerate_inhibitors(Manager *m) { k = manager_add_inhibitor(m, de->d_name, &i); if (k < 0) { - log_notice_errno(k, "Couldn't add inhibitor %s: %m", de->d_name); - r = k; + r = log_warning_errno(k, "Couldn't add inhibitor %s, ignoring: %m", de->d_name); continue; } @@ -1157,8 +1145,15 @@ static int manager_startup(Manager *m) { HASHMAP_FOREACH(session, m->sessions, i) (void) session_start(session, NULL, NULL); - HASHMAP_FOREACH(inhibitor, m->inhibitors, i) - inhibitor_start(inhibitor); + HASHMAP_FOREACH(inhibitor, m->inhibitors, i) { + (void) inhibitor_start(inhibitor); + + /* Let's see if the inhibitor is dead now, then remove it */ + if (inhibitor_is_orphan(inhibitor)) { + inhibitor_stop(inhibitor); + inhibitor_free(inhibitor); + } + } HASHMAP_FOREACH(button, m->buttons, i) button_check_switches(button);