From 183e0738427b83667512276a3e8c10274c0824cc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 May 2016 18:57:15 +0200 Subject: [PATCH 1/7] logind: enforce a limit on current user sessions We really should put limits on all resources we manage, hence add one to the number of concurrent sessions, too. This was previously unbounded, hence set a relatively high limit of 8K by default. Note that most PAM setups will actually invoke pam_systemd prefixed with "-", so that the return code of pam_systemd is ignored, and the login attempt succeeds anyway. On systems like this the session will be created but is not tracked by systemd. --- man/logind.conf.xml | 9 +++++++++ src/login/logind-dbus.c | 23 +++++++++++++++++++++++ src/login/logind-gperf.gperf | 1 + src/login/logind.c | 1 + src/login/logind.conf.in | 1 + src/login/logind.h | 1 + 6 files changed, 36 insertions(+) diff --git a/man/logind.conf.xml b/man/logind.conf.xml index 6ba35414bef..405dcf90414 100644 --- a/man/logind.conf.xml +++ b/man/logind.conf.xml @@ -296,6 +296,15 @@ memory as is needed. + + SessionsMax= + + Controls the maximum number of concurrent user sessions to manage. Defaults to 8192 + (8K). Depending on how the pam_systemd.so module is included in the PAM stack + configuration, further login sessions will either be refused, or permitted but not tracked by + systemd-logind. + + UserTasksMax= diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index a281f99a343..5067e5d1a84 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -265,6 +265,24 @@ static int property_get_docked( return sd_bus_message_append(reply, "b", manager_is_docked_or_external_displays(m)); } +static int property_get_current_sessions( + sd_bus *bus, + const char *path, + const char *interface, + const char *property, + sd_bus_message *reply, + void *userdata, + sd_bus_error *error) { + + Manager *m = userdata; + + assert(bus); + assert(reply); + assert(m); + + return sd_bus_message_append(reply, "t", (uint64_t) hashmap_size(m->sessions)); +} + static int method_get_session(sd_bus_message *message, void *userdata, sd_bus_error *error) { _cleanup_free_ char *p = NULL; Manager *m = userdata; @@ -725,6 +743,9 @@ static int method_create_session(sd_bus_message *message, void *userdata, sd_bus m->seat0->positions[vtnr]->class != SESSION_GREETER) return sd_bus_error_setf(error, BUS_ERROR_SESSION_BUSY, "Already occupied by a session"); + if (hashmap_size(m->sessions) >= m->sessions_max) + return sd_bus_error_setf(error, SD_BUS_ERROR_LIMITS_EXCEEDED, "Maximum number of sessions (%" PRIu64 ") reached, refusing further sessions.", m->sessions_max); + audit_session_from_pid(leader, &audit_id); if (audit_id > 0) { /* Keep our session IDs and the audit session IDs in sync */ @@ -2512,6 +2533,8 @@ const sd_bus_vtable manager_vtable[] = { SD_BUS_PROPERTY("PreparingForSleep", "b", property_get_preparing, 0, 0), SD_BUS_PROPERTY("ScheduledShutdown", "(st)", property_get_scheduled_shutdown, 0, 0), SD_BUS_PROPERTY("Docked", "b", property_get_docked, 0, 0), + SD_BUS_PROPERTY("SessionsMax", "t", NULL, offsetof(Manager, sessions_max), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("NCurrentSessions", "t", property_get_current_sessions, 0, 0), SD_BUS_METHOD("GetSession", "s", "o", method_get_session, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("GetSessionByPID", "u", "o", method_get_session_by_pid, SD_BUS_VTABLE_UNPRIVILEGED), diff --git a/src/login/logind-gperf.gperf b/src/login/logind-gperf.gperf index 8552c464cca..1d576812608 100644 --- a/src/login/logind-gperf.gperf +++ b/src/login/logind-gperf.gperf @@ -34,4 +34,5 @@ Login.IdleAction, config_parse_handle_action, 0, offsetof(Manag Login.IdleActionSec, config_parse_sec, 0, offsetof(Manager, idle_action_usec) Login.RuntimeDirectorySize, config_parse_tmpfs_size, 0, offsetof(Manager, runtime_dir_size) Login.RemoveIPC, config_parse_bool, 0, offsetof(Manager, remove_ipc) +Login.SessionsMax, config_parse_uint64, 0, offsetof(Manager, sessions_max) Login.UserTasksMax, config_parse_uint64, 0, offsetof(Manager, user_tasks_max) diff --git a/src/login/logind.c b/src/login/logind.c index a48e2fc61ec..39f53cab360 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -63,6 +63,7 @@ static void manager_reset_config(Manager *m) { m->runtime_dir_size = PAGE_ALIGN((size_t) (physical_memory() / 10)); /* 10% */ m->user_tasks_max = UINT64_C(12288); + m->sessions_max = UINT64_C(8192); m->kill_user_processes = KILL_USER_PROCESSES; diff --git a/src/login/logind.conf.in b/src/login/logind.conf.in index 3c96def45d9..62842186255 100644 --- a/src/login/logind.conf.in +++ b/src/login/logind.conf.in @@ -32,4 +32,5 @@ #IdleActionSec=30min #RuntimeDirectorySize=10% #RemoveIPC=yes +#SessionsMax=8192 #UserTasksMax=12288 diff --git a/src/login/logind.h b/src/login/logind.h index 6748af3c07e..23c3e2963a6 100644 --- a/src/login/logind.h +++ b/src/login/logind.h @@ -133,6 +133,7 @@ struct Manager { size_t runtime_dir_size; uint64_t user_tasks_max; + uint64_t sessions_max; }; int manager_add_device(Manager *m, const char *sysfs, bool master, Device **_device); From 89f193fac8e1b85936e32455ed7a5ea6b43d6508 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 May 2016 19:01:46 +0200 Subject: [PATCH 2/7] update TODO --- TODO | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/TODO b/TODO index 515dfba3c99..b44670ad9d4 100644 --- a/TODO +++ b/TODO @@ -33,6 +33,10 @@ Janitorial Clean-ups: Features: +* maybe: pid1: replace cgroups agent transport by AF_UNIX/SOCK_DGRAM, so that + we aren't hit by socket backlog exhaustion on the dbus AF_UNIX/SOCK_STREAM + socket + * journalctl: make sure -f ends when the container indicated by -M terminates * rework fopen_temporary() to make use of open_tmpfile_linkable() (problem: the From e11544a8305ab9dea097c74bb16e296150c9cc10 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 May 2016 19:01:56 +0200 Subject: [PATCH 3/7] logind: process session/inhibitor fds at higher priority Let's make sure we process session and inhibitor pipe fds (that signal sessions/inhibtors going away) at a higher priority than new bus calls that might create new sessions or inhibitors. This helps ensuring that the number of open sessions stays minimal. --- src/login/logind-inhibit.c | 2 +- src/login/logind-session.c | 4 +++- src/login/logind.c | 2 +- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/login/logind-inhibit.c b/src/login/logind-inhibit.c index a0e3ba2b7cd..6c78e0dddc9 100644 --- a/src/login/logind-inhibit.c +++ b/src/login/logind-inhibit.c @@ -317,7 +317,7 @@ int inhibitor_create_fifo(Inhibitor *i) { if (r < 0) return r; - r = sd_event_source_set_priority(i->event_source, SD_EVENT_PRIORITY_IDLE); + r = sd_event_source_set_priority(i->event_source, SD_EVENT_PRIORITY_IDLE-10); if (r < 0) return r; } diff --git a/src/login/logind-session.c b/src/login/logind-session.c index a8b1d5943db..d2f1f7bc62b 100644 --- a/src/login/logind-session.c +++ b/src/login/logind-session.c @@ -897,7 +897,9 @@ int session_create_fifo(Session *s) { if (r < 0) return r; - r = sd_event_source_set_priority(s->fifo_event_source, SD_EVENT_PRIORITY_IDLE); + /* Let's make sure we noticed dead sessions before we process new bus requests (which might create new + * sessions). */ + r = sd_event_source_set_priority(s->fifo_event_source, SD_EVENT_PRIORITY_NORMAL-10); if (r < 0) return r; } diff --git a/src/login/logind.c b/src/login/logind.c index 39f53cab360..64bd1ca5820 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -687,7 +687,7 @@ static int manager_connect_bus(Manager *m) { if (r < 0) return log_error_errno(r, "Failed to register name: %m"); - r = sd_bus_attach_event(m->bus, m->event, 0); + r = sd_bus_attach_event(m->bus, m->event, SD_EVENT_PRIORITY_NORMAL); if (r < 0) return log_error_errno(r, "Failed to attach bus to event loop: %m"); From 91ab7b01f883e75257189f71006ea1e7261a88dd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 May 2016 19:05:18 +0200 Subject: [PATCH 4/7] logind: don't include session lists in PropertyChanged messages If we have a lot of simultaneous sessions we really shouldn't send the full list of active sessions with each PropertyChanged message for user and seat objects, as that can become quite substantial data, we probably shouldn't dump on the bus on each login and logout. Note that the global list of sessions doesn't send out changes like this either, it only supports requesting the session list with ListSessions(). If cients want to get notified about sessions coming and going they should subscribe to SessionNew and SessionRemoved signals, and clients generally do that already. This is kind of an API break, but then again the fact that this was included was never documented. --- src/login/logind-seat-dbus.c | 2 +- src/login/logind-user-dbus.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/login/logind-seat-dbus.c b/src/login/logind-seat-dbus.c index 3cee10d009a..f934a5326a2 100644 --- a/src/login/logind-seat-dbus.c +++ b/src/login/logind-seat-dbus.c @@ -306,7 +306,7 @@ const sd_bus_vtable seat_vtable[] = { SD_BUS_PROPERTY("CanMultiSession", "b", property_get_can_multi_session, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("CanTTY", "b", property_get_can_tty, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("CanGraphical", "b", property_get_can_graphical, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), - SD_BUS_PROPERTY("Sessions", "a(so)", property_get_sessions, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_PROPERTY("Sessions", "a(so)", property_get_sessions, 0, 0), SD_BUS_PROPERTY("IdleHint", "b", property_get_idle_hint, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("IdleSinceHint", "t", property_get_idle_since_hint, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("IdleSinceHintMonotonic", "t", property_get_idle_since_hint, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), diff --git a/src/login/logind-user-dbus.c b/src/login/logind-user-dbus.c index b73f9ea69e8..af6392e0257 100644 --- a/src/login/logind-user-dbus.c +++ b/src/login/logind-user-dbus.c @@ -245,7 +245,7 @@ const sd_bus_vtable user_vtable[] = { SD_BUS_PROPERTY("Slice", "s", NULL, offsetof(User, slice), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Display", "(so)", property_get_display, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("State", "s", property_get_state, 0, 0), - SD_BUS_PROPERTY("Sessions", "a(so)", property_get_sessions, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), + SD_BUS_PROPERTY("Sessions", "a(so)", property_get_sessions, 0, 0), SD_BUS_PROPERTY("IdleHint", "b", property_get_idle_hint, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("IdleSinceHint", "t", property_get_idle_since_hint, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), SD_BUS_PROPERTY("IdleSinceHintMonotonic", "t", property_get_idle_since_hint, 0, SD_BUS_VTABLE_PROPERTY_EMITS_CHANGE), From 6d97d3c648e590fe05a5fd12d06e0ddb9dc9da2f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 May 2016 19:22:30 +0200 Subject: [PATCH 5/7] logind: expose more configuration settings as bus properties --- src/login/logind-dbus.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index 5067e5d1a84..d249bff2d9c 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -2533,8 +2533,11 @@ const sd_bus_vtable manager_vtable[] = { SD_BUS_PROPERTY("PreparingForSleep", "b", property_get_preparing, 0, 0), SD_BUS_PROPERTY("ScheduledShutdown", "(st)", property_get_scheduled_shutdown, 0, 0), SD_BUS_PROPERTY("Docked", "b", property_get_docked, 0, 0), + SD_BUS_PROPERTY("RemoveIPC", "b", bus_property_get_bool, offsetof(Manager, remove_ipc), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("RuntimeDirectorySize", "t", bus_property_get_size, offsetof(Manager, runtime_dir_size), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("SessionsMax", "t", NULL, offsetof(Manager, sessions_max), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("NCurrentSessions", "t", property_get_current_sessions, 0, 0), + SD_BUS_PROPERTY("UserTasksMax", "t", NULL, offsetof(Manager, user_tasks_max), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_METHOD("GetSession", "s", "o", method_get_session, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("GetSessionByPID", "u", "o", method_get_session_by_pid, SD_BUS_VTABLE_UNPRIVILEGED), From c5a11ae268cf4188caf74d1acfd506a606e85967 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 4 May 2016 19:40:05 +0200 Subject: [PATCH 6/7] logind: enforce a limit on inhibitors we hand out For similar reasons as the recent addition of a limit on sessions. Note that we don't enforce a limit on inhibitors per-user currently, but there's an implicit one, since each inhibitor takes up one fd, and fds are limited via RLIMIT_NOFILE, and the limit on the number of processes per user. --- man/logind.conf.xml | 7 +++++++ src/login/logind-dbus.c | 23 +++++++++++++++++++++++ src/login/logind-gperf.gperf | 1 + src/login/logind.c | 1 + src/login/logind.conf.in | 1 + src/login/logind.h | 1 + 6 files changed, 34 insertions(+) diff --git a/man/logind.conf.xml b/man/logind.conf.xml index 405dcf90414..fe92277a1f8 100644 --- a/man/logind.conf.xml +++ b/man/logind.conf.xml @@ -296,6 +296,13 @@ memory as is needed. + + InhibitorsMax= + + Controls the maximum number of concurrent inhibitors to permit. Defaults to 8192 + (8K). + + SessionsMax= diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c index d249bff2d9c..0a84d75e249 100644 --- a/src/login/logind-dbus.c +++ b/src/login/logind-dbus.c @@ -283,6 +283,24 @@ static int property_get_current_sessions( return sd_bus_message_append(reply, "t", (uint64_t) hashmap_size(m->sessions)); } +static int property_get_current_inhibitors( + sd_bus *bus, + const char *path, + const char *interface, + const char *property, + sd_bus_message *reply, + void *userdata, + sd_bus_error *error) { + + Manager *m = userdata; + + assert(bus); + assert(reply); + assert(m); + + return sd_bus_message_append(reply, "t", (uint64_t) hashmap_size(m->inhibitors)); +} + static int method_get_session(sd_bus_message *message, void *userdata, sd_bus_error *error) { _cleanup_free_ char *p = NULL; Manager *m = userdata; @@ -2463,6 +2481,9 @@ static int method_inhibit(sd_bus_message *message, void *userdata, sd_bus_error if (r < 0) return r; + if (hashmap_size(m->inhibitors) >= m->inhibitors_max) + return sd_bus_error_setf(error, SD_BUS_ERROR_LIMITS_EXCEEDED, "Maximum number of inhibitors (%" PRIu64 ") reached, refusing further inhibitors.", m->inhibitors_max); + do { id = mfree(id); @@ -2535,6 +2556,8 @@ const sd_bus_vtable manager_vtable[] = { SD_BUS_PROPERTY("Docked", "b", property_get_docked, 0, 0), SD_BUS_PROPERTY("RemoveIPC", "b", bus_property_get_bool, offsetof(Manager, remove_ipc), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RuntimeDirectorySize", "t", bus_property_get_size, offsetof(Manager, runtime_dir_size), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("InhibitorsMax", "t", NULL, offsetof(Manager, inhibitors_max), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("NCurrentInhibitors", "t", property_get_current_inhibitors, 0, 0), SD_BUS_PROPERTY("SessionsMax", "t", NULL, offsetof(Manager, sessions_max), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("NCurrentSessions", "t", property_get_current_sessions, 0, 0), SD_BUS_PROPERTY("UserTasksMax", "t", NULL, offsetof(Manager, user_tasks_max), SD_BUS_VTABLE_PROPERTY_CONST), diff --git a/src/login/logind-gperf.gperf b/src/login/logind-gperf.gperf index 1d576812608..6bd08adc055 100644 --- a/src/login/logind-gperf.gperf +++ b/src/login/logind-gperf.gperf @@ -34,5 +34,6 @@ Login.IdleAction, config_parse_handle_action, 0, offsetof(Manag Login.IdleActionSec, config_parse_sec, 0, offsetof(Manager, idle_action_usec) Login.RuntimeDirectorySize, config_parse_tmpfs_size, 0, offsetof(Manager, runtime_dir_size) Login.RemoveIPC, config_parse_bool, 0, offsetof(Manager, remove_ipc) +Login.InhibitorsMax, config_parse_uint64, 0, offsetof(Manager, inhibitors_max) Login.SessionsMax, config_parse_uint64, 0, offsetof(Manager, sessions_max) Login.UserTasksMax, config_parse_uint64, 0, offsetof(Manager, user_tasks_max) diff --git a/src/login/logind.c b/src/login/logind.c index 64bd1ca5820..1cbc8f9fcc1 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -64,6 +64,7 @@ static void manager_reset_config(Manager *m) { m->runtime_dir_size = PAGE_ALIGN((size_t) (physical_memory() / 10)); /* 10% */ m->user_tasks_max = UINT64_C(12288); m->sessions_max = UINT64_C(8192); + m->inhibitors_max = UINT64_C(8192); m->kill_user_processes = KILL_USER_PROCESSES; diff --git a/src/login/logind.conf.in b/src/login/logind.conf.in index 62842186255..32c0844cb65 100644 --- a/src/login/logind.conf.in +++ b/src/login/logind.conf.in @@ -32,5 +32,6 @@ #IdleActionSec=30min #RuntimeDirectorySize=10% #RemoveIPC=yes +#InhibitorsMax=8192 #SessionsMax=8192 #UserTasksMax=12288 diff --git a/src/login/logind.h b/src/login/logind.h index 23c3e2963a6..90431eb4b08 100644 --- a/src/login/logind.h +++ b/src/login/logind.h @@ -134,6 +134,7 @@ struct Manager { size_t runtime_dir_size; uint64_t user_tasks_max; uint64_t sessions_max; + uint64_t inhibitors_max; }; int manager_add_device(Manager *m, const char *sysfs, bool master, Device **_device); From 64b5689647c7e12522ead1108037afe0b58b0dfd Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 5 May 2016 22:49:25 +0200 Subject: [PATCH 7/7] logind: drop pointless UINT64_C() macro use --- src/login/logind.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/login/logind.c b/src/login/logind.c index 1cbc8f9fcc1..caf149cfb78 100644 --- a/src/login/logind.c +++ b/src/login/logind.c @@ -62,9 +62,9 @@ static void manager_reset_config(Manager *m) { m->idle_action = HANDLE_IGNORE; m->runtime_dir_size = PAGE_ALIGN((size_t) (physical_memory() / 10)); /* 10% */ - m->user_tasks_max = UINT64_C(12288); - m->sessions_max = UINT64_C(8192); - m->inhibitors_max = UINT64_C(8192); + m->user_tasks_max = 12288; + m->sessions_max = 8192; + m->inhibitors_max = 8192; m->kill_user_processes = KILL_USER_PROCESSES;