[systemd-commits] 3 commits - src/login
Lennart Poettering
lennart at kemper.freedesktop.org
Tue Sep 17 09:33:37 PDT 2013
src/login/logind-dbus.c | 25 +++++++++++++++
src/login/logind-session-dbus.c | 42 ++++++++++++++++++++++++++
src/login/logind-session.c | 56 ++++++++++++++++++++++++++++++-----
src/login/logind-session.h | 6 +++
src/login/logind.c | 64 +++++++++++++++++++++++++++++++++++++++-
src/login/logind.h | 4 ++
6 files changed, 189 insertions(+), 8 deletions(-)
New commits:
commit ab60f2ffb1a1fe2024aea077b3f42c3653bf1df1
Author: David Herrmann <dh.herrmann at gmail.com>
Date: Tue Sep 17 17:39:57 2013 +0200
logind: make Session.Activate() lazy
Currently, Activate() calls chvt(), which does an ioctl(VT_ACTIVATE) and
immediately calls seat_set_active(). However, VTs are allowed to prevent
being deactivated. Therefore, logind cannot be sure the VT_ACTIVATE call
was actually successful.
Furthermore, compositors often need to clean up their devices before they
acknowledge the VT switch. The immediate call to seat_set_active() may
modify underlying ACLs, though. Thus, some compositors may fail cleaning
up their stuff. Moreover, the compositor being switched to (if listening
to logind instead of VTs) will not be able to activate its devices if the
old VT still has them active.
We could simply add an VT_WAITACTIVE call, which blocks until the given VT
is active. However, this can block forever if the compositor hangs.
So to fix this, we make Activate() lazy. That is, it only schedules a
session-switch but does not wait for it to complete. The caller can no
longer rely on it being immediate. Instead, a caller is required to wait
for the PropertiesChanged signal and read the "Active" field.
We could make Activate() wait asynchronously for the session-switch to
complete and then send the return-message afterwards. However, this would
add a lot of state-tracking with no real gain:
1) Sessions normally don't care whether Activate() was actually
successful as they currently _must_ wait for the VT activation to do
anything for real.
2) Error messages for failed session switches can be printed by logind
instead of the session issuing Activate().
3) Sessions that require synchronous Activate() calls can simply issue
the call and then wait for "Active" properties to change. This also
allows them to implement their own timeout.
This change prepares for multi-session on seats without VTs. Forced VT
switches are always bad as compositors cannot perform any cleanup. This
isn't strictly required, but may lead to loss of information and ambiguous
error messages.
So for multi-session on seats without VTs, we must wait for the current
session to clean-up before finalizing the session-switch. This requires
Activate() to be lazy as we cannot block here.
Note that we can always implement a timeout which allows us to guarantee
the session switch to happen. Nevertheless, this calls for a lazy
Activate().
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index fe5fa27..fa8b515 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -360,8 +360,6 @@ int session_load(Session *s) {
}
int session_activate(Session *s) {
- int r;
-
assert(s);
assert(s->user);
@@ -376,11 +374,7 @@ int session_activate(Session *s) {
assert(seat_is_vtconsole(s->seat));
- r = chvt(s->vtnr);
- if (r < 0)
- return r;
-
- return seat_set_active(s->seat, s);
+ return chvt(s->vtnr);
}
static int session_link_x11_socket(Session *s) {
commit ae5e06bda24ebbb2ac00741738ad3a872fc577a5
Author: David Herrmann <dh.herrmann at gmail.com>
Date: Tue Sep 17 17:39:56 2013 +0200
logind: add session controllers
A session usually has only a single compositor or other application that
controls graphics and input devices on it. To avoid multiple applications
from hijacking each other's devices or even using the devices in parallel,
we add session controllers.
A session controller is an application that manages a session. Specific
API calls may be limited to controllers to avoid others from getting
unprivileged access to restricted resources. A session becomes a
controller by calling the RequestControl() dbus API call. It can drop it
via ReleaseControl().
logind tracks bus-names to release the controller once an application
closes the bus. We use the new bus-name tracking to do that. Note that
during ReleaseControl() we need to check whether some other session also
tracks the name before we remove it from the bus-name tracking list.
Currently, we only allow one controller at a time. However, the public API
does not enforce this restriction. So if it makes sense, we can allow
multiple controllers in parallel later. Or we can add a "scope" parameter,
which allows a different controller for graphics-devices, sound-devices
and whatever you want.
Note that currently you get -EBUSY if there is already a controller. You
can force the RequestControl() call (root-only) to drop the current
controller and recover the session during an emergency. To recover a seat,
this is not needed, though. You can simply create a new session or
force-activate it.
To become a session controller, a dbus caller must either be root or the
same user as the user of the session. This allows us to run a session
compositor as user and we no longer need any CAP_SYS_ADMIN.
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 5decf81..113a2b7 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -2473,8 +2473,16 @@ DBusHandlerResult bus_message_filter(
goto finish;
}
+ /* drop all controllers owned by this name */
if (*old && !*new && (key = hashmap_remove(m->busnames, old))) {
+ Session *session;
+ Iterator i;
+
free(key);
+
+ HASHMAP_FOREACH(session, m->sessions, i)
+ if (session_is_controller(session, old))
+ session_drop_controller(session);
}
}
diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c
index 2cc4d85..35bf448 100644
--- a/src/login/logind-session-dbus.c
+++ b/src/login/logind-session-dbus.c
@@ -40,6 +40,10 @@
" <arg name=\"who\" type=\"s\"/>\n" \
" <arg name=\"signal\" type=\"s\"/>\n" \
" </method>\n" \
+ " <method name=\"TakeControl\"/>\n" \
+ " <arg name=\"force\" type=\"b\"/>\n" \
+ " </method>\n" \
+ " <method name=\"ReleaseControl\"/>\n" \
" <signal name=\"Lock\"/>\n" \
" <signal name=\"Unlock\"/>\n" \
" <property name=\"Id\" type=\"s\" access=\"read\"/>\n" \
@@ -366,6 +370,44 @@ static DBusHandlerResult session_message_dispatch(
if (!reply)
goto oom;
+ } else if (dbus_message_is_method_call(message, "org.freedesktop.login1.Session", "TakeControl")) {
+ dbus_bool_t force;
+ unsigned long ul;
+
+ if (!dbus_message_get_args(
+ message,
+ &error,
+ DBUS_TYPE_BOOLEAN, &force,
+ DBUS_TYPE_INVALID))
+ return bus_send_error_reply(connection, message, &error, -EINVAL);
+
+ ul = dbus_bus_get_unix_user(connection, dbus_message_get_sender(message), &error);
+ if (ul == (unsigned long) -1)
+ return bus_send_error_reply(connection, message, &error, -EIO);
+
+ if (ul != 0 && (force || ul != s->user->uid))
+ return bus_send_error_reply(connection, message, NULL, -EPERM);
+
+ r = session_set_controller(s, bus_message_get_sender_with_fallback(message), force);
+ if (r < 0)
+ return bus_send_error_reply(connection, message, NULL, r);
+
+ reply = dbus_message_new_method_return(message);
+ if (!reply)
+ goto oom;
+
+ } else if (dbus_message_is_method_call(message, "org.freedesktop.login1.Session", "ReleaseControl")) {
+ const char *sender = bus_message_get_sender_with_fallback(message);
+
+ if (!session_is_controller(s, sender))
+ return bus_send_error_reply(connection, message, NULL, -EPERM);
+
+ session_drop_controller(s);
+
+ reply = dbus_message_new_method_return(message);
+ if (!reply)
+ goto oom;
+
} else {
const BusBoundProperties bps[] = {
{ "org.freedesktop.login1.Session", bus_login_session_properties, s },
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 2d22a68..fe5fa27 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -73,6 +73,8 @@ void session_free(Session *s) {
if (s->in_gc_queue)
LIST_REMOVE(Session, gc_queue, s->manager->session_gc_queue, s);
+ session_drop_controller(s);
+
if (s->user) {
LIST_REMOVE(Session, sessions_by_user, s->user->sessions, s);
@@ -918,6 +920,52 @@ int session_kill(Session *s, KillWho who, int signo) {
return manager_kill_unit(s->manager, s->scope, who, signo, NULL);
}
+bool session_is_controller(Session *s, const char *sender)
+{
+ assert(s);
+
+ return streq_ptr(s->controller, sender);
+}
+
+int session_set_controller(Session *s, const char *sender, bool force) {
+ char *t;
+ int r;
+
+ assert(s);
+ assert(sender);
+
+ if (session_is_controller(s, sender))
+ return 0;
+ if (s->controller && !force)
+ return -EBUSY;
+
+ t = strdup(sender);
+ if (!t)
+ return -ENOMEM;
+
+ r = manager_watch_busname(s->manager, sender);
+ if (r) {
+ free(t);
+ return r;
+ }
+
+ session_drop_controller(s);
+
+ s->controller = t;
+ return 0;
+}
+
+void session_drop_controller(Session *s) {
+ assert(s);
+
+ if (!s->controller)
+ return;
+
+ manager_drop_busname(s->manager, s->controller);
+ free(s->controller);
+ s->controller = NULL;
+}
+
static const char* const session_state_table[_SESSION_STATE_MAX] = {
[SESSION_OPENING] = "opening",
[SESSION_ONLINE] = "online",
diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index 9cf6485..839fb23 100644
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -106,6 +106,8 @@ struct Session {
DBusMessage *create_message;
+ char *controller;
+
LIST_FIELDS(Session, sessions_by_user);
LIST_FIELDS(Session, sessions_by_seat);
@@ -154,3 +156,7 @@ SessionClass session_class_from_string(const char *s) _pure_;
const char *kill_who_to_string(KillWho k) _const_;
KillWho kill_who_from_string(const char *s) _pure_;
+
+bool session_is_controller(Session *s, const char *sender);
+int session_set_controller(Session *s, const char *sender, bool force);
+void session_drop_controller(Session *s);
diff --git a/src/login/logind.c b/src/login/logind.c
index 89e4eee..c99c284 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -391,11 +391,21 @@ int manager_watch_busname(Manager *m, const char *name) {
}
void manager_drop_busname(Manager *m, const char *name) {
+ Session *session;
+ Iterator i;
char *key;
assert(m);
assert(name);
+ if (!hashmap_get(m->busnames, name))
+ return;
+
+ /* keep it if the name still owns a controller */
+ HASHMAP_FOREACH(session, m->sessions, i)
+ if (session_is_controller(session, name))
+ return;
+
key = hashmap_remove(m->busnames, name);
if (key)
free(key);
commit e8b212fe56f7f4e1778474777a7a2959244d0047
Author: David Herrmann <dh.herrmann at gmail.com>
Date: Tue Sep 17 17:39:55 2013 +0200
logind: add infrastructure to watch busnames
If we want to track bus-names to allow exclusive resource-access, we need
a way to get notified when a bus-name is gone. We make logind watch for
NameOwnerChanged dbus events and check whether the name is currently
watched. If it is, we remove it from the watch-list (notification for
other objects can be added in follow-up patches).
diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index d052e74..5decf81 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -2459,6 +2459,23 @@ DBusHandlerResult bus_message_filter(
HASHMAP_FOREACH(session, m->sessions, i)
session_add_to_gc_queue(session);
}
+
+ } else if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS, "NameOwnerChanged")) {
+ const char *name, *old, *new;
+ char *key;
+
+ if (!dbus_message_get_args(message, &error,
+ DBUS_TYPE_STRING, &name,
+ DBUS_TYPE_STRING, &old,
+ DBUS_TYPE_STRING, &new,
+ DBUS_TYPE_INVALID)) {
+ log_error("Failed to parse NameOwnerChanged message: %s", bus_error_message(&error));
+ goto finish;
+ }
+
+ if (*old && !*new && (key = hashmap_remove(m->busnames, old))) {
+ free(key);
+ }
}
finish:
diff --git a/src/login/logind.c b/src/login/logind.c
index 29019c2..89e4eee 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -74,6 +74,7 @@ Manager *manager_new(void) {
m->users = hashmap_new(trivial_hash_func, trivial_compare_func);
m->inhibitors = hashmap_new(string_hash_func, string_compare_func);
m->buttons = hashmap_new(string_hash_func, string_compare_func);
+ m->busnames = hashmap_new(string_hash_func, string_compare_func);
m->user_units = hashmap_new(string_hash_func, string_compare_func);
m->session_units = hashmap_new(string_hash_func, string_compare_func);
@@ -82,7 +83,7 @@ Manager *manager_new(void) {
m->inhibitor_fds = hashmap_new(trivial_hash_func, trivial_compare_func);
m->button_fds = hashmap_new(trivial_hash_func, trivial_compare_func);
- if (!m->devices || !m->seats || !m->sessions || !m->users || !m->inhibitors || !m->buttons ||
+ if (!m->devices || !m->seats || !m->sessions || !m->users || !m->inhibitors || !m->buttons || !m->busnames ||
!m->user_units || !m->session_units ||
!m->session_fds || !m->inhibitor_fds || !m->button_fds) {
manager_free(m);
@@ -111,6 +112,7 @@ void manager_free(Manager *m) {
Seat *s;
Inhibitor *i;
Button *b;
+ char *n;
assert(m);
@@ -132,12 +134,16 @@ void manager_free(Manager *m) {
while ((b = hashmap_first(m->buttons)))
button_free(b);
+ while ((n = hashmap_first(m->busnames)))
+ free(hashmap_remove(m->busnames, n));
+
hashmap_free(m->devices);
hashmap_free(m->seats);
hashmap_free(m->sessions);
hashmap_free(m->users);
hashmap_free(m->inhibitors);
hashmap_free(m->buttons);
+ hashmap_free(m->busnames);
hashmap_free(m->user_units);
hashmap_free(m->session_units);
@@ -361,6 +367,40 @@ int manager_add_button(Manager *m, const char *name, Button **_button) {
return 0;
}
+int manager_watch_busname(Manager *m, const char *name) {
+ char *n;
+ int r;
+
+ assert(m);
+ assert(name);
+
+ if (hashmap_get(m->busnames, name))
+ return 0;
+
+ n = strdup(name);
+ if (!n)
+ return -ENOMEM;
+
+ r = hashmap_put(m->busnames, n, n);
+ if (r < 0) {
+ free(n);
+ return r;
+ }
+
+ return 0;
+}
+
+void manager_drop_busname(Manager *m, const char *name) {
+ char *key;
+
+ assert(m);
+ assert(name);
+
+ key = hashmap_remove(m->busnames, name);
+ if (key)
+ free(key);
+}
+
int manager_process_seat_device(Manager *m, struct udev_device *d) {
Device *device;
int r;
@@ -1045,6 +1085,18 @@ static int manager_connect_bus(Manager *m) {
dbus_bus_add_match(m->bus,
"type='signal',"
+ "sender='"DBUS_SERVICE_DBUS"',"
+ "interface='"DBUS_INTERFACE_DBUS"',"
+ "member='NameOwnerChanged',"
+ "path='"DBUS_PATH_DBUS"'",
+ &error);
+ if (dbus_error_is_set(&error)) {
+ log_error("Failed to add match for NameOwnerChanged: %s", bus_error_message(&error));
+ dbus_error_free(&error);
+ }
+
+ dbus_bus_add_match(m->bus,
+ "type='signal',"
"sender='org.freedesktop.systemd1',"
"interface='org.freedesktop.systemd1.Manager',"
"member='JobRemoved',"
diff --git a/src/login/logind.h b/src/login/logind.h
index 1a97351..a76936d 100644
--- a/src/login/logind.h
+++ b/src/login/logind.h
@@ -51,6 +51,7 @@ struct Manager {
Hashmap *users;
Hashmap *inhibitors;
Hashmap *buttons;
+ Hashmap *busnames;
LIST_HEAD(Seat, seat_gc_queue);
LIST_HEAD(Session, session_gc_queue);
@@ -190,3 +191,6 @@ int manager_unit_is_active(Manager *manager, const char *unit);
/* gperf lookup function */
const struct ConfigPerfItem* logind_gperf_lookup(const char *key, unsigned length);
+
+int manager_watch_busname(Manager *manager, const char *name);
+void manager_drop_busname(Manager *manager, const char *name);
More information about the systemd-commits
mailing list