Discussion:
[PATCH 00/10] Device Management for systemd-logind
(too old to reply)
David Herrmann
2013-08-25 12:46:05 UTC
Permalink
Hi

This series implements device management for logind. A session can now request
device access directly via logind dbus APIs. It extends the
org.freedesktop.login1.Session interface. The already existing interface is
described at:
http://www.freedesktop.org/wiki/Software/systemd/logind/

The reason for this series and the basic idea is discussed on:
http://dvdhrm.wordpress.com/2013/08/25/sane-session-switching/

If someone is not familiar with session-management and, more importantly,
session-switching, I summarized it at:
http://dvdhrm.wordpress.com/2013/08/24/session-management-on-linux/
http://dvdhrm.wordpress.com/2013/08/24/how-vt-switching-works/

Ok, feel free to dismiss the rest of this mail and look directly at the patches.
The commit-messages explain everything in detail. For all who are not familiar
with the systemd code-base, a summary can be found below.

Feedback welcome! I am open for any suggestions.

Tests, examples and more will be available at https://github.com/dvdhrm/libnovt
once the API stabilized.

Regards
David

CC: wayland-***@fdo for weston-launch development.



The following calls are introduced by this series. They are added to the logind
dbus API for sessions. Hence, each call implicitly operates on a given session
and thus does not have to be passed as argument.

These methods are added:

RequestDevice(const char *node, int *fd_out, bool *paused_out):
logind tries to open a device node in /dev/ (@node) for the caller. It
checks that it is assigned to the seat of the caller, has the uaccess
flags set and then opens the fd and passes it back via @fd_out. The
@paused_out field will notify the caller whether the device is currently
paused or active.
Remarks:
- you cannot open a node twice in parallel
- you must use the canonical device-node (the one from devtmpfs) and
returned by udev_device_get_devnode(). No other nodes can be
supported; clients could trigger OOM otherwise
- logind keeps a copy of @fd internally to revoke access once the
session becomes inactive

ReleaseDevice(const char *node):
logind closes the device that was previously requested via
RequestDevice(). It revokes access from the fd and closes it internally.
The caller still has their own fd, but it will be useless. But they
obviously need to close() it theirself.
You may call RequestDevice() on the same node again, once you released
it.

As long as a process owns a device, it is notified via dbus signals about state
changes:

PauseDevice(const char *node, const char *type):
logind sends this event whenever the device is paused or released. @node
contains the device-node passed to RequestDevice() and is unique. @type
is "gone" if the device was closed or removed from the seat. A
compositor normally gets a udev "remove" event at the same time. They
should treat it similarly.
@type is "force" if the device was forcibly paused by logind. A
compositor normally gets EACCES or EPERM simultaneously from any
syscalls it tries on the fd. Hence, it is adviced to handle EACCES/EPERM
the same as a PauseDevice("force") signal from systemd.
Last but not least, there is "pause" as @type. This is a kind request by
logind to the compositor to pause the device. A compositor gets a short
timeout to react to this event, cleanup everything and acknowledge the
signal via a call to PauseDeviceComplete(). If it doesn't react in a
timely manner, a PauseDevice("force") event will be sent.

ResumeDevice(const char *node, int *fd_out):
For every device requested via RequestDevice(), logind sends this event
whenever the device is resumed. It also puts a new file-descriptor into
@fd_out. A compositor is advised to close its old fd and use the new
one. For some device-types (namely evdev) it *must* use the new fd, as
the old one is revoked. For other device-types (namely DRM) both will
actually be the same as access can be restored.
Compositors are allowed to rely on this behavior for DRM. That is, if
it's a DRM device, they can close the new fd and keep the old one
(they're actually just dup()'ed). This allows them to retain their DRM
state.

Methods to acknowledge some signals are:

PauseDeviceComplete(const char *node):
As mentioned above in PauseDevice, this is a method that can be called
by compositors to react to a PauseDevice("pause") request. After it is
called, the given device will be paused.

Note that device-management is available on all sessions regardless their type
and state. Moreover, device-state follows session-state but might also be paused
for other reasons that are currently not defined. This means, whenever a session
is inactive, all devices on this session are inactive, too. However, when a
session is active, a device might stay inactive (for instance if reactivation
failed). But logind guarantees that a device can never be used by anyone else
than the foreground session. Compositors can rely on that for security reasons.

logind itselfs takes care of revoking device access for inactive sessions
(synchronized with session-switches!). It also tries to resume every device
when a session is activated. But session-devices must not be used to watch
session state! A compositor has to use the PropertiesChanged() signal plus the
"Active" property of sessions for that! Session-devices do not replace this! On
sessions with VTs, this is obviously replaced by the VT_SETMODE interface as
usual.

Now additionally to the interface mentioned above, this series introduces
session-controllers. These try to prevent multiple compositors from running in
parallel. That is, when a compositor starts up, it calls RequestControl() on the
session in question. If there is already another compositor running, this will
fail.
The new device-management functions are limited to the active controller. No
other functions make use of controllers.

Note that the RequestControl() call might get a "scope" argument. So you can
have a controller with scope "graphics" and one for scope "sound", for example.
On each scope only a single controller is allowed, but the scopes don't
interfere. So logind makes sure that RequestDevice() on a graphics or input
device requires the "graphics" scope.

But now the API:

RequestControl(bool force):
For now this misses a "scope" argument, so it currently implies
"graphics" scope. This function will make the caller the new controller
of the given session. logind watches the system bus and automatically
drops the controller once it disconnects.
If there is already a controller for the given scope, this returns
EBUSY. If @force is true, the active controller will be dropped and the
caller will get the new controller.
This function is restricted to callers with the same UID as the "User"
of the given session. If @force is true, the caller must be root.
So this call in combination with RequestDevice() allows us to run a
compositor in a session as normal user. Yay!

DropControl():
Drop control again. If the caller is not the current controller, this
does nothing. Note that this call is optional. logind watches the bus
for disconnect events and invokes this implicitly if a controller exits.

David Herrmann (10):
logind: listen actively for session devices
logind: add infrastructure to watch busnames
logind: add session controllers
logind: make Session.Activate() lazy
logind: introduce session-devices
logind: rename vtconsole to seat0
logind: fix seat_can_tty() to check for VTs
logind: fix session_activate(vtnr = 0)
logind: extract has_vts() from can_multi_session()
logind: implement generic multi-session

Makefile.am | 2 +
src/login/logind-dbus.c | 35 ++-
src/login/logind-device.c | 39 ++-
src/login/logind-device.h | 5 +-
src/login/logind-seat.c | 73 ++++--
src/login/logind-seat.h | 6 +-
src/login/logind-session-dbus.c | 156 ++++++++++++
src/login/logind-session-device.c | 517 ++++++++++++++++++++++++++++++++++++++
src/login/logind-session-device.h | 58 +++++
src/login/logind-session.c | 112 ++++++++-
src/login/logind-session.h | 8 +
src/login/logind.c | 151 +++++++++--
src/login/logind.h | 12 +-
13 files changed, 1109 insertions(+), 65 deletions(-)
create mode 100644 src/login/logind-session-device.c
create mode 100644 src/login/logind-session-device.h
--
1.8.3.4
David Herrmann
2013-08-25 12:46:06 UTC
Permalink
Session compositors need access to fbdev, DRM and evdev devices if they
control a session. To make logind pass them to sessions, we need to
listen for them actively.

However, we avoid creating new seats for non master-of-seat devices. Only
once a seat is created, we start remembering all other session devices. If
the last master-device is removed (even if there are other non-master
devices still available), we destroy the seat. This is the current
behavior, but we need to explicitly implement it now as there may be
non-master devices in the seat->devices list.

Unlike master devices, we don't care whether our list of non-master
devices is complete. We don't export this list but use it only as cache if
sessions request these devices. Hence, if a session requests a device that
is not in the list, we will simply look it up. However, once a session
requested a device, we must be notified of "remove" udev events. So we
must link the devices somehow into the device-list.

Regarding the implementation, we now sort the device list by the "master"
flag. This guarantees that master devices are at the front and non-master
devices at the tail of the list. Thus, we can easily test whether a seat
has a master device attached.
---
src/login/logind-device.c | 35 ++++++++++++++++++---
src/login/logind-device.h | 3 +-
src/login/logind-seat.c | 11 +++++--
src/login/logind-seat.h | 1 +
src/login/logind.c | 79 +++++++++++++++++++++++++++++++++++++++++------
src/login/logind.h | 6 ++--
6 files changed, 116 insertions(+), 19 deletions(-)

diff --git a/src/login/logind-device.c b/src/login/logind-device.c
index 51b1535..a9a9633 100644
--- a/src/login/logind-device.c
+++ b/src/login/logind-device.c
@@ -25,7 +25,7 @@
#include "logind-device.h"
#include "util.h"

-Device* device_new(Manager *m, const char *sysfs) {
+Device* device_new(Manager *m, const char *sysfs, bool master) {
Device *d;

assert(m);
@@ -48,6 +48,7 @@ Device* device_new(Manager *m, const char *sysfs) {
}

d->manager = m;
+ d->master = master;
dual_timestamp_get(&d->timestamp);

return d;
@@ -75,11 +76,16 @@ void device_detach(Device *d) {
LIST_REMOVE(Device, devices, d->seat->devices, d);
d->seat = NULL;

- seat_add_to_gc_queue(s);
- seat_send_changed(s, "CanGraphical\0");
+ if (!seat_has_master_device(s)) {
+ seat_add_to_gc_queue(s);
+ seat_send_changed(s, "CanGraphical\0");
+ }
}

void device_attach(Device *d, Seat *s) {
+ Device *i;
+ bool had_master;
+
assert(d);
assert(s);

@@ -90,7 +96,26 @@ void device_attach(Device *d, Seat *s) {
device_detach(d);

d->seat = s;
- LIST_PREPEND(Device, devices, s->devices, d);
+ had_master = seat_has_master_device(s);
+
+ /* We keep the device list sorted by the "master" flag. That is, master
+ * devices are at the front, other devices at the tail. As there is no
+ * way to easily add devices at the list-tail, we need to iterate the
+ * list to find the first non-master device when adding non-master
+ * devices. We assume there is only a few (normally 1) master devices
+ * per seat, so we iterate only a few times. */
+
+ if (d->master || !s->devices) {
+ LIST_PREPEND(Device, devices, s->devices, d);
+ } else {
+ LIST_FOREACH(devices, i, s->devices) {
+ if (!i->devices_next || !i->master) {
+ LIST_INSERT_AFTER(Device, devices, s->devices, i, d);
+ break;
+ }
+ }
+ }

- seat_send_changed(s, "CanGraphical\0");
+ if (!had_master && d->master)
+ seat_send_changed(s, "CanGraphical\0");
}
diff --git a/src/login/logind-device.h b/src/login/logind-device.h
index 3b15356..315f0e6 100644
--- a/src/login/logind-device.h
+++ b/src/login/logind-device.h
@@ -33,13 +33,14 @@ struct Device {

char *sysfs;
Seat *seat;
+ bool master;

dual_timestamp timestamp;

LIST_FIELDS(struct Device, devices);
};

-Device* device_new(Manager *m, const char *sysfs);
+Device* device_new(Manager *m, const char *sysfs, bool master);
void device_free(Device *d);
void device_attach(Device *d, Seat *s);
void device_detach(Device *d);
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index 470d08b..2c60b8a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -448,10 +448,17 @@ bool seat_can_tty(Seat *s) {
return seat_is_vtconsole(s);
}

+bool seat_has_master_device(Seat *s) {
+ assert(s);
+
+ /* device list is ordered by "master" flag */
+ return !!s->devices && s->devices->master;
+}
+
bool seat_can_graphical(Seat *s) {
assert(s);

- return !!s->devices;
+ return seat_has_master_device(s);
}

int seat_get_idle_hint(Seat *s, dual_timestamp *t) {
@@ -499,7 +506,7 @@ int seat_check_gc(Seat *s, bool drop_not_started) {
if (seat_is_vtconsole(s))
return 1;

- return !!s->devices;
+ return seat_has_master_device(s);
}

void seat_add_to_gc_queue(Seat *s) {
diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index c8ab17f..bd5390f 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -63,6 +63,7 @@ int seat_attach_session(Seat *s, Session *session);
bool seat_is_vtconsole(Seat *s);
bool seat_can_multi_session(Seat *s);
bool seat_can_tty(Seat *s);
+bool seat_has_master_device(Seat *s);
bool seat_can_graphical(Seat *s);

int seat_get_idle_hint(Seat *s, dual_timestamp *t);
diff --git a/src/login/logind.c b/src/login/logind.c
index 0002d26..54fb391 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -151,6 +151,8 @@ void manager_free(Manager *m) {

if (m->udev_seat_monitor)
udev_monitor_unref(m->udev_seat_monitor);
+ if (m->udev_device_monitor)
+ udev_monitor_unref(m->udev_device_monitor);
if (m->udev_vcsa_monitor)
udev_monitor_unref(m->udev_vcsa_monitor);
if (m->udev_button_monitor)
@@ -184,7 +186,7 @@ void manager_free(Manager *m) {
free(m);
}

-int manager_add_device(Manager *m, const char *sysfs, Device **_device) {
+int manager_add_device(Manager *m, const char *sysfs, bool master, Device **_device) {
Device *d;

assert(m);
@@ -195,10 +197,13 @@ int manager_add_device(Manager *m, const char *sysfs, Device **_device) {
if (_device)
*_device = d;

+ /* we support adding master-flags, but not removing them */
+ d->master = d->master || master;
+
return 0;
}

- d = device_new(m, sysfs);
+ d = device_new(m, sysfs, master);
if (!d)
return -ENOMEM;

@@ -373,7 +378,8 @@ int manager_process_seat_device(Manager *m, struct udev_device *d) {

} else {
const char *sn;
- Seat *seat;
+ Seat *seat = NULL;
+ bool master;

sn = udev_device_get_property_value(d, "ID_SEAT");
if (isempty(sn))
@@ -384,16 +390,23 @@ int manager_process_seat_device(Manager *m, struct udev_device *d) {
return 0;
}

- r = manager_add_device(m, udev_device_get_syspath(d), &device);
+ /* ignore non-master devices for unknown seats */
+ master = udev_device_has_tag(d, "master-of-seat");
+ if (!master && !(seat = hashmap_get(m->seats, sn)))
+ return 0;
+
+ r = manager_add_device(m, udev_device_get_syspath(d), master, &device);
if (r < 0)
return r;

- r = manager_add_seat(m, sn, &seat);
- if (r < 0) {
- if (!device->seat)
- device_free(device);
+ if (!seat) {
+ r = manager_add_seat(m, sn, &seat);
+ if (r < 0) {
+ if (!device->seat)
+ device_free(device);

- return r;
+ return r;
+ }
}

device_attach(device, seat);
@@ -756,6 +769,22 @@ int manager_dispatch_seat_udev(Manager *m) {
return r;
}

+static int manager_dispatch_device_udev(Manager *m) {
+ struct udev_device *d;
+ int r;
+
+ assert(m);
+
+ d = udev_monitor_receive_device(m->udev_device_monitor);
+ if (!d)
+ return -ENOMEM;
+
+ r = manager_process_seat_device(m, d);
+ udev_device_unref(d);
+
+ return r;
+}
+
int manager_dispatch_vcsa_udev(Manager *m) {
struct udev_device *d;
int r = 0;
@@ -1143,6 +1172,7 @@ static int manager_connect_udev(Manager *m) {

assert(m);
assert(!m->udev_seat_monitor);
+ assert(!m->udev_device_monitor);
assert(!m->udev_vcsa_monitor);
assert(!m->udev_button_monitor);

@@ -1163,6 +1193,33 @@ static int manager_connect_udev(Manager *m) {
if (epoll_ctl(m->epoll_fd, EPOLL_CTL_ADD, m->udev_seat_fd, &ev) < 0)
return -errno;

+ m->udev_device_monitor = udev_monitor_new_from_netlink(m->udev, "udev");
+ if (!m->udev_device_monitor)
+ return -ENOMEM;
+
+ r = udev_monitor_filter_add_match_subsystem_devtype(m->udev_device_monitor, "input", NULL);
+ if (r < 0)
+ return r;
+
+ r = udev_monitor_filter_add_match_subsystem_devtype(m->udev_device_monitor, "graphics", NULL);
+ if (r < 0)
+ return r;
+
+ r = udev_monitor_filter_add_match_subsystem_devtype(m->udev_device_monitor, "drm", NULL);
+ if (r < 0)
+ return r;
+
+ r = udev_monitor_enable_receiving(m->udev_device_monitor);
+ if (r < 0)
+ return r;
+
+ m->udev_device_fd = udev_monitor_get_fd(m->udev_device_monitor);
+ zero(ev);
+ ev.events = EPOLLIN;
+ ev.data.u32 = FD_DEVICE_UDEV;
+ if (epoll_ctl(m->epoll_fd, EPOLL_CTL_ADD, m->udev_device_fd, &ev) < 0)
+ return -errno;
+
/* Don't watch keys if nobody cares */
if (m->handle_power_key != HANDLE_IGNORE ||
m->handle_suspend_key != HANDLE_IGNORE ||
@@ -1522,6 +1579,10 @@ int manager_run(Manager *m) {
manager_dispatch_seat_udev(m);
break;

+ case FD_DEVICE_UDEV:
+ manager_dispatch_device_udev(m);
+ break;
+
case FD_VCSA_UDEV:
manager_dispatch_vcsa_udev(m);
break;
diff --git a/src/login/logind.h b/src/login/logind.h
index e9838a8..1a97351 100644
--- a/src/login/logind.h
+++ b/src/login/logind.h
@@ -57,9 +57,10 @@ struct Manager {
LIST_HEAD(User, user_gc_queue);

struct udev *udev;
- struct udev_monitor *udev_seat_monitor, *udev_vcsa_monitor, *udev_button_monitor;
+ struct udev_monitor *udev_seat_monitor, *udev_device_monitor, *udev_vcsa_monitor, *udev_button_monitor;

int udev_seat_fd;
+ int udev_device_fd;
int udev_vcsa_fd;
int udev_button_fd;

@@ -121,6 +122,7 @@ struct Manager {

enum {
FD_SEAT_UDEV,
+ FD_DEVICE_UDEV,
FD_VCSA_UDEV,
FD_BUTTON_UDEV,
FD_CONSOLE,
@@ -132,7 +134,7 @@ enum {
Manager *manager_new(void);
void manager_free(Manager *m);

-int manager_add_device(Manager *m, const char *sysfs, Device **_device);
+int manager_add_device(Manager *m, const char *sysfs, bool master, Device **_device);
int manager_add_button(Manager *m, const char *name, Button **_button);
int manager_add_seat(Manager *m, const char *id, Seat **_seat);
int manager_add_session(Manager *m, const char *id, Session **_session);
--
1.8.3.4
David Herrmann
2013-08-25 12:46:07 UTC
Permalink
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).
---
src/login/logind-dbus.c | 17 ++++++++++++++++
src/login/logind.c | 54 ++++++++++++++++++++++++++++++++++++++++++++++++-
src/login/logind.h | 4 ++++
3 files changed, 74 insertions(+), 1 deletion(-)

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 345df9f..eb62d28 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -2458,6 +2458,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 54fb391..f4cef54 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;
@@ -1039,6 +1079,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);
--
1.8.3.4
David Herrmann
2013-08-25 12:46:08 UTC
Permalink
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.
---
src/login/logind-dbus.c | 8 +++++++
src/login/logind-session-dbus.c | 42 ++++++++++++++++++++++++++++++++++++
src/login/logind-session.c | 48 +++++++++++++++++++++++++++++++++++++++++
src/login/logind-session.h | 6 ++++++
src/login/logind.c | 10 +++++++++
5 files changed, 114 insertions(+)

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index eb62d28..a703e59 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -2472,8 +2472,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..b8b32cd 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=\"RequestControl\"/>\n" \
+ " <arg name=\"force\" type=\"b\"/>\n" \
+ " </method>\n" \
+ " <method name=\"DropControl\"/>\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", "RequestControl")) {
+ 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", "DropControl")) {
+ 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 1fea474..6c6a2c8 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -72,6 +72,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);

@@ -925,6 +927,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 edaae8d..411a1b1 100644
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -105,6 +105,8 @@ struct Session {

DBusMessage *create_message;

+ char *controller;
+
LIST_FIELDS(Session, sessions_by_user);
LIST_FIELDS(Session, sessions_by_seat);

@@ -153,3 +155,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 f4cef54..ad3ab81 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);
--
1.8.3.4
David Herrmann
2013-08-25 12:46:09 UTC
Permalink
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().
---
src/login/logind-session.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 6c6a2c8..a11804a 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -359,8 +359,6 @@ int session_load(Session *s) {
}

int session_activate(Session *s) {
- int r;
-
assert(s);
assert(s->user);

@@ -375,11 +373,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) {
--
1.8.3.4
David Herrmann
2013-08-25 12:46:10 UTC
Permalink
A session-device is a device that is bound to a seat and used by a
session-controller to run the session. This currently includes DRM, fbdev
and evdev devices. A session-device can be created via RequestDevice() on
the dbus API of the session. You can drop it via ReleaseDevice() again.
Once the session is destroyed or you drop control of the session, all
session-devices are automatically destroyed.

Session devices mimic the session "active" state. A device can be
active/running or inactive/paused. Whenever a session is not the active
session, no session-device of it can be active. That is, if a session is
not in foreground, all session-devices are paused.
Whenever a session becomes active, all devices are resumed/activated by
logind. If it fails, a device may stay paused.

With every session-device you request, you also get a file-descriptor
back. logind keeps a copy of this fd and uses kernel specific calls to
pause/resume the file-descriptors. For example, a DRM fd is muted
by logind as long as a given session is not active. Hence, the fd of the
application is also muted. Once the session gets active, logind unmutes
the fd and the application will get DRM access again.
This, however, requires kernel support. DRM devices provide DRM-Master for
synchronization, evdev devices have EVIOCREVOKE (pending on
linux-input-ML). fbdev devices do not provide such synchronization methods
(and never will).
Note that for evdev devices, we call EVIOCREVOKE once a session gets
inactive. However, this cannot be undone (the fd is still valid but mostly
unusable). So we reopen a new fd once the session is activated and send it
together with the ResumeDevice() signal.

With this infrastructure in place, compositors can now run without
CAP_SYS_ADMIN (that is, without being root). They use RequestControl() to
acquire a session and listen for devices via udev_monitor. For every
device-node they want to open, they call RequestDevice() on logind. This
returns a fd which they can use now. They no longer have to open the
devices themselves or call any privileged ioctls. This is all done by
logind.
Session-switches are still bound to VTs. Hence, compositors will get
notified via the usual VT mechanisms and can cleanup their state. Once the
VT switch is acknowledged as usual, logind will get notified via sysfs and
pause the old-session's devices and resume the devices of the new session.

To allow using this infrastructure with systems without VTs, we provide
notification signals. logind sends PauseDevice("force") dbus signals to
the current session controller for every device that it pauses. And it
sends ResumeDevice signals for every device that it resumes. For
seats with VTs this is sent _after_ the VT switch is acknowledged. Because
the compositor already acknowledged that it cleaned-up all devices.
However, for seats without VTs, this is used to notify the active
compositor that the session is about to be deactivated. That is, logind
sends PauseDevice("force") for each active device and then performs the
session-switch. The session-switch changes the "Active" property of the
session which can be monitored by the compositor. The new session is
activated and the ResumeDevice events are sent.

For seats without VTs, this is a forced session-switch. As this is not
backwards-compatible (xserver actually crashes, weston drops the related
devices, ..) we also provide an acknowledged session-switch. Note that
this is never used for sessions with VTs. You use the acknowledged
VT-switch on these seats.

An acknowledged session switch sends PauseDevice("pause") instead of
PauseDevice("force") to the active session. It schedules a short timeout
and waits for the session to acknowledge each of them with
PauseDeviceComplete(). Once all are acknowledged, or the session ran out
of time, a PauseDevice("force") is sent for all remaining active devices
and the session switch is performed.
Note that this is only partially implemented, yet, as we don't allow
multi-session without VTs, yet. A follow up commit will hook it up and
implemented the acknowledgements+timeout.

The implementation is quite simple. We use device-nodes exclusively to
identify devices on the bus. On RequestDevice() we retrieve the
udev_device from the device-node and search for an existing "Device"
object. If no exists, we create it. This guarantees us that we are
notified whenever the device changes seats or is removed.

We create a new SessionDevice object and link it to the related Session
and Device. Session->devices is a hashtable to lookup SessionDevice
objects via device-node. Device->session_devices is a linked list so we
can release all linked session-devices once a device vanishes.

Now we only have to hook this up in seat_set_active() so we correctly
change device states during session-switches. As mentioned earlier, these
are forced state-changes as VTs are currently used exclusively for
multi-session implementations.

Everything else are hooks to release all session-devices once the
controller changes or a session is closed or removed.
---
Makefile.am | 2 +
src/login/logind-device.c | 4 +
src/login/logind-device.h | 2 +
src/login/logind-seat.c | 7 +-
src/login/logind-session-dbus.c | 114 +++++++++
src/login/logind-session-device.c | 489 ++++++++++++++++++++++++++++++++++++++
src/login/logind-session-device.h | 57 +++++
src/login/logind-session.c | 27 +++
src/login/logind-session.h | 2 +
9 files changed, 703 insertions(+), 1 deletion(-)
create mode 100644 src/login/logind-session-device.c
create mode 100644 src/login/logind-session-device.h

diff --git a/Makefile.am b/Makefile.am
index fd38e82..c5a8800 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -3748,6 +3748,8 @@ libsystemd_logind_core_la_SOURCES = \
src/login/logind-seat.h \
src/login/logind-session.c \
src/login/logind-session.h \
+ src/login/logind-session-device.c \
+ src/login/logind-session-device.h \
src/login/logind-user.c \
src/login/logind-user.h \
src/login/logind-inhibit.c \
diff --git a/src/login/logind-device.c b/src/login/logind-device.c
index a9a9633..9f432ea 100644
--- a/src/login/logind-device.c
+++ b/src/login/logind-device.c
@@ -67,11 +67,15 @@ void device_free(Device *d) {

void device_detach(Device *d) {
Seat *s;
+ SessionDevice *sd;
assert(d);

if (!d->seat)
return;

+ while ((sd = d->session_devices))
+ session_device_free(sd);
+
s = d->seat;
LIST_REMOVE(Device, devices, d->seat->devices, d);
d->seat = NULL;
diff --git a/src/login/logind-device.h b/src/login/logind-device.h
index 315f0e6..fa6eda7 100644
--- a/src/login/logind-device.h
+++ b/src/login/logind-device.h
@@ -27,6 +27,7 @@ typedef struct Device Device;
#include "util.h"
#include "logind.h"
#include "logind-seat.h"
+#include "logind-session-device.h"

struct Device {
Manager *manager;
@@ -38,6 +39,7 @@ struct Device {
dual_timestamp timestamp;

LIST_FIELDS(struct Device, devices);
+ LIST_HEAD(SessionDevice, session_devices);
};

Device* device_new(Manager *m, const char *sysfs, bool master);
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index 2c60b8a..dcaf0ac 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -246,10 +246,15 @@ int seat_set_active(Seat *s, Session *session) {
old_active = s->active;
s->active = session;

+ if (old_active)
+ session_device_pause_all(old_active);
+
seat_apply_acls(s, old_active);

- if (session && session->started)
+ if (session && session->started) {
session_send_changed(session, "Active\0");
+ session_device_resume_all(session);
+ }

if (!session || session->started)
seat_send_changed(s, "ActiveSession\0");
diff --git a/src/login/logind-session-dbus.c b/src/login/logind-session-dbus.c
index b8b32cd..2725d4c 100644
--- a/src/login/logind-session-dbus.c
+++ b/src/login/logind-session-dbus.c
@@ -24,6 +24,7 @@

#include "logind.h"
#include "logind-session.h"
+#include "logind-session-device.h"
#include "dbus-common.h"
#include "util.h"

@@ -44,6 +45,25 @@
" <arg name=\"force\" type=\"b\"/>\n" \
" </method>\n" \
" <method name=\"DropControl\"/>\n" \
+ " <method name=\"RequestDevice\">\n" \
+ " <arg name=\"node\" type=\"s\" direction=\"in\"/>\n" \
+ " <arg name=\"fd\" type=\"h\" direction=\"out\"/>\n" \
+ " <arg name=\"paused\" type=\"b\" direction=\"out\"/>\n" \
+ " </method>\n" \
+ " <method name=\"ReleaseDevice\">\n" \
+ " <arg name=\"node\" type=\"s\"/>\n" \
+ " </method>\n" \
+ " <method name=\"PauseDeviceComplete\">\n" \
+ " <arg name=\"node\" type=\"s\"/>\n" \
+ " </method>\n" \
+ " <signal name=\"PauseDevice\">\n" \
+ " <arg name=\"node\" type=\"s\"/>\n" \
+ " <arg name=\"type\" type=\"s\"/>\n" \
+ " </signal>\n" \
+ " <signal name=\"ResumeDevice\">\n" \
+ " <arg name=\"node\" type=\"s\"/>\n" \
+ " <arg name=\"fd\" type=\"h\"/>\n" \
+ " </signal>\n" \
" <signal name=\"Lock\"/>\n" \
" <signal name=\"Unlock\"/>\n" \
" <property name=\"Id\" type=\"s\" access=\"read\"/>\n" \
@@ -408,6 +428,100 @@ static DBusHandlerResult session_message_dispatch(
if (!reply)
goto oom;

+ } else if (dbus_message_is_method_call(message, "org.freedesktop.login1.Session", "RequestDevice")) {
+ const char *node;
+ SessionDevice *sd;
+ bool b;
+ dbus_bool_t paused;
+
+ if (!session_is_controller(s, bus_message_get_sender_with_fallback(message)))
+ return bus_send_error_reply(connection, message, NULL, -EPERM);
+
+ if (!dbus_message_get_args(
+ message,
+ &error,
+ DBUS_TYPE_STRING, &node,
+ DBUS_TYPE_INVALID))
+ return bus_send_error_reply(connection, message, &error, -EINVAL);
+
+ sd = hashmap_get(s->devices, node);
+ if (sd) {
+ /* We don't allow retrieving a device multiple times.
+ * The related ReleaseDevice call is not ref-counted.
+ * The caller should use dup() if it requires more than
+ * one fd (it would be functionally equivalent). */
+ return bus_send_error_reply(connection, message, &error, -EBUSY);
+ }
+
+ r = session_device_new(s, node, &sd);
+ if (r < 0)
+ return bus_send_error_reply(connection, message, NULL, r);
+
+ reply = dbus_message_new_method_return(message);
+ if (!reply) {
+ session_device_free(sd);
+ goto oom;
+ }
+
+ paused = !sd->active;
+ b = dbus_message_append_args(
+ reply,
+ DBUS_TYPE_UNIX_FD, &sd->fd,
+ DBUS_TYPE_BOOLEAN, &paused,
+ DBUS_TYPE_INVALID);
+ if (!b) {
+ session_device_free(sd);
+ return bus_send_error_reply(connection, message, NULL, -ENOMEM);
+ }
+
+ } else if (dbus_message_is_method_call(message, "org.freedesktop.login1.Session", "ReleaseDevice")) {
+ const char *node;
+ SessionDevice *sd;
+
+ if (!session_is_controller(s, bus_message_get_sender_with_fallback(message)))
+ return bus_send_error_reply(connection, message, NULL, -EPERM);
+
+ if (!dbus_message_get_args(
+ message,
+ &error,
+ DBUS_TYPE_STRING, &node,
+ DBUS_TYPE_INVALID))
+ return bus_send_error_reply(connection, message, &error, -EINVAL);
+
+ sd = hashmap_get(s->devices, node);
+ if (!sd)
+ return bus_send_error_reply(connection, message, NULL, -ENODEV);
+
+ session_device_free(sd);
+
+ reply = dbus_message_new_method_return(message);
+ if (!reply)
+ goto oom;
+
+ } else if (dbus_message_is_method_call(message, "org.freedesktop.login1.Session", "PauseDeviceComplete")) {
+ const char *node;
+ SessionDevice *sd;
+
+ if (!session_is_controller(s, bus_message_get_sender_with_fallback(message)))
+ return bus_send_error_reply(connection, message, NULL, -EPERM);
+
+ if (!dbus_message_get_args(
+ message,
+ &error,
+ DBUS_TYPE_STRING, &node,
+ DBUS_TYPE_INVALID))
+ return bus_send_error_reply(connection, message, &error, -EINVAL);
+
+ sd = hashmap_get(s->devices, node);
+ if (!sd)
+ return bus_send_error_reply(connection, message, NULL, -ENODEV);
+
+ session_device_complete_pause(sd);
+
+ 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-device.c b/src/login/logind-session-device.c
new file mode 100644
index 0000000..7c6b07f
--- /dev/null
+++ b/src/login/logind-session-device.c
@@ -0,0 +1,489 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+/***
+ This file is part of systemd.
+
+ Copyright 2013 David Herrmann
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+#include <assert.h>
+#include <fcntl.h>
+#include <libudev.h>
+#include <linux/input.h>
+#include <linux/ioctl.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/stat.h>
+#include <sys/types.h>
+#include <unistd.h>
+
+#include "dbus-common.h"
+#include "logind-session-device.h"
+#include "util.h"
+
+enum SessionDeviceNotifications {
+ SESSION_DEVICE_RESUME,
+ SESSION_DEVICE_TRY_PAUSE,
+ SESSION_DEVICE_PAUSE,
+ SESSION_DEVICE_RELEASE,
+};
+
+static void session_device_notify(SessionDevice *sd, enum SessionDeviceNotifications type) {
+ _cleanup_dbus_message_unref_ DBusMessage *m = NULL;
+ _cleanup_free_ char *path;
+ const char *t = NULL;
+
+ assert(sd);
+
+ if (!sd->session->controller)
+ return;
+
+ path = session_bus_path(sd->session);
+ if (!path)
+ return;
+
+ m = dbus_message_new_signal(path,
+ "org.freedesktop.login1.Session",
+ (type == SESSION_DEVICE_RESUME) ? "ResumeDevice" : "PauseDevice");
+ if (!m)
+ return;
+
+ if (!dbus_message_set_destination(m, sd->session->controller))
+ return;
+
+ switch (type) {
+ case SESSION_DEVICE_RESUME:
+ if (!dbus_message_append_args(m,
+ DBUS_TYPE_STRING, &sd->node,
+ DBUS_TYPE_UNIX_FD, &sd->fd,
+ DBUS_TYPE_INVALID))
+ return;
+ break;
+ case SESSION_DEVICE_TRY_PAUSE:
+ t = "pause";
+ break;
+ case SESSION_DEVICE_PAUSE:
+ t = "force";
+ break;
+ case SESSION_DEVICE_RELEASE:
+ t = "gone";
+ break;
+ default:
+ return;
+ }
+
+ if (t && !dbus_message_append_args(m,
+ DBUS_TYPE_STRING, &sd->node,
+ DBUS_TYPE_STRING, &t,
+ DBUS_TYPE_INVALID))
+ return;
+
+ dbus_connection_send(sd->session->manager->bus, m, NULL);
+}
+
+static int sd_eviocrevoke(int fd)
+{
+ static bool warned;
+ int r;
+
+#ifndef EVIOCREVOKE
+# define EVIOCREVOKE _IOW('E', 0x91, int)
+#endif
+
+ assert(fd >= 0);
+
+ r = ioctl(fd, EVIOCREVOKE, 1);
+ if (r < 0) {
+ r = -errno;
+ if (r == -ENOSYS && !warned) {
+ warned = true;
+ log_warning("kernel does not support evdev-revocation");
+ }
+ }
+
+ return 0;
+}
+
+static int sd_drmsetmaster(int fd)
+{
+ int r;
+
+#ifndef DRM_IOCTL_SET_MASTER
+# define DRM_IOCTL_SET_MASTER _IO('d', 0x1e)
+#endif
+
+ assert(fd >= 0);
+
+ r = ioctl(fd, DRM_IOCTL_SET_MASTER, 0);
+ if (r < 0)
+ return -errno;
+
+ return 0;
+}
+
+static int sd_drmdropmaster(int fd)
+{
+ int r;
+
+#ifndef DRM_IOCTL_DROP_MASTER
+# define DRM_IOCTL_DROP_MASTER _IO('d', 0x1f)
+#endif
+
+ assert(fd >= 0);
+
+ r = ioctl(fd, DRM_IOCTL_DROP_MASTER, 0);
+ if (r < 0)
+ return -errno;
+
+ return 0;
+}
+
+static int session_device_open(SessionDevice *sd, bool active) {
+ int fd;
+
+ assert(sd->type != DEVICE_TYPE_UNKNOWN);
+
+ /* open device and try to get an udev_device from it */
+ fd = open(sd->node, O_RDWR|O_CLOEXEC|O_NOCTTY|O_NONBLOCK);
+ if (fd < 0)
+ return -errno;
+
+ switch (sd->type) {
+ case DEVICE_TYPE_DRM:
+ if (active) {
+ sd_drmsetmaster(fd);
+ } else {
+ /* DRM-Master is granted to the first user who opens a
+ * device automatically (ughh, racy!). Hence, we just
+ * drop DRM-Master in case we were the first. */
+ sd_drmdropmaster(fd);
+ }
+ break;
+ case DEVICE_TYPE_EVDEV:
+ if (!active)
+ sd_eviocrevoke(fd);
+ break;
+ case DEVICE_TYPE_FBDEV:
+ case DEVICE_TYPE_UNKNOWN:
+ default:
+ /* fallback for devices wihout synchronizations */
+ break;
+ }
+
+ return fd;
+}
+
+static int session_device_start(SessionDevice *sd) {
+ int r;
+
+ assert(sd);
+ assert(session_is_active(sd->session));
+
+ if (sd->active)
+ return 0;
+
+ switch (sd->type) {
+ case DEVICE_TYPE_DRM:
+ /* Device is kept open. Simply call drmSetMaster() and hope
+ * there is no-one else. In case it fails, we keep the device
+ * paused. Maybe at some point we have a drmStealMaster(). */
+ r = sd_drmsetmaster(sd->fd);
+ if (r < 0)
+ return r;
+ break;
+ case DEVICE_TYPE_EVDEV:
+ /* Evdev devices are revoked while inactive. Reopen it and we
+ * are fine. */
+ r = session_device_open(sd, true);
+ if (r < 0)
+ return r;
+ close_nointr_nofail(sd->fd);
+ sd->fd = r;
+ break;
+ case DEVICE_TYPE_FBDEV:
+ /* fbdev devices have no way to synchronize access. Moreover,
+ * they mostly operate through mmaps() without any pageflips
+ * and modesetting, so there is no way for us to prevent access
+ * but tear down mmaps.
+ * That would be quite expensive to do on a per-fd context. So
+ * ignore legcy fbdev and let its users feel the pain they asked
+ * for when deciding for fbdev. */
+ case DEVICE_TYPE_UNKNOWN:
+ default:
+ /* fallback for devices wihout synchronizations */
+ break;
+ }
+
+ sd->active = true;
+ return 0;
+}
+
+static void session_device_stop(SessionDevice *sd) {
+ assert(sd);
+
+ if (!sd->active)
+ return;
+
+ switch (sd->type) {
+ case DEVICE_TYPE_DRM:
+ /* On DRM devices we simply drop DRM-Master but keep it open.
+ * This allows the user to keep resources allocated. The
+ * CAP_SYS_ADMIN restriction to DRM-Master prevents users from
+ * circumventing this. */
+ sd_drmdropmaster(sd->fd);
+ break;
+ case DEVICE_TYPE_EVDEV:
+ /* Revoke access on evdev file-descriptors during deactivation.
+ * This will basically prevent any operations on the fd and
+ * cannot be undone. Good side is: it needs no CAP_SYS_ADMIN
+ * protection this way. */
+ sd_eviocrevoke(sd->fd);
+ break;
+ case DEVICE_TYPE_FBDEV:
+ case DEVICE_TYPE_UNKNOWN:
+ default:
+ /* fallback for devices without synchronization */
+ break;
+ }
+
+ sd->active = false;
+}
+
+static enum DeviceType detect_device_type(struct udev_device *dev) {
+ const char *sysname;
+ dev_t dnum;
+ enum DeviceType type;
+
+ dnum = udev_device_get_devnum(dev);
+ sysname = udev_device_get_sysname(dev);
+ type = DEVICE_TYPE_UNKNOWN;
+
+ if (major(dnum) == 29) {
+ if (startswith(sysname, "fb"))
+ type = DEVICE_TYPE_FBDEV;
+ } else if (major(dnum) == 226) {
+ if (startswith(sysname, "card"))
+ type = DEVICE_TYPE_DRM;
+ } else if (major(dnum) == 13) {
+ if (startswith(sysname, "event"))
+ type = DEVICE_TYPE_EVDEV;
+ }
+
+ return type;
+}
+
+static int session_device_verify(SessionDevice *sd, const char *node) {
+ struct stat st;
+ struct udev_device *dev, *p = NULL;
+ const char *sp, *real_node;
+ int r;
+
+ r = stat(node, &st);
+ if (r < 0)
+ return -errno;
+ else if (!S_ISCHR(st.st_mode))
+ return -ENODEV;
+
+ dev = udev_device_new_from_devnum(sd->session->manager->udev, 'c', st.st_rdev);
+ if (!dev)
+ return -ENODEV;
+ sp = udev_device_get_syspath(dev);
+
+ /* Only allow access to "uaccess" tagged devices! */
+ if (!udev_device_has_tag(dev, "uaccess")) {
+ r = -EPERM;
+ goto err_dev;
+ }
+
+ /* Only allow opening the _real_ device node. This is the node provided
+ * by devtmpfs! Issue a readlink() if you are not sure. We cannot allow
+ * any path here as user-space could otherwise trigger OOM by requesting
+ * duplicates. */
+ real_node = udev_device_get_devnode(dev);
+ if (!streq_ptr(real_node, node)) {
+ r = -EINVAL;
+ goto err_dev;
+ }
+
+ /* detect device type so we can find the correct sysfs parent */
+ sd->type = detect_device_type(dev);
+ if (sd->type == DEVICE_TYPE_UNKNOWN) {
+ r = -ENODEV;
+ goto err_dev;
+ } else if (sd->type == DEVICE_TYPE_EVDEV) {
+ /* for evdev devices we need the parent node as device */
+ p = dev;
+ dev = udev_device_get_parent_with_subsystem_devtype(p, "input", NULL);
+ if (!dev) {
+ r = -ENODEV;
+ goto err_dev;
+ }
+ sp = udev_device_get_syspath(dev);
+ } else if (sd->type != DEVICE_TYPE_FBDEV &&
+ sd->type != DEVICE_TYPE_DRM) {
+ /* Prevent opening unsupported devices. Especially devices of
+ * subsystem "input" must be opened via the evdev node as
+ * we require EVIOCREVOKE. */
+ r = -ENODEV;
+ goto err_dev;
+ }
+
+ /* search for an existing seat device and return it if available */
+ sd->device = hashmap_get(sd->session->manager->devices, sp);
+ if (sd->device) {
+ if (sd->device->seat != sd->session->seat) {
+ r = -EPERM;
+ goto err_dev;
+ }
+
+ udev_device_unref(p ? : dev);
+ return 0;
+ }
+
+ /* The caller might have gotten the udev event before we were able to
+ * process it. Hence, fake the "add" event and let the logind-manager
+ * handle the new device. */
+ r = manager_process_seat_device(sd->session->manager, dev);
+ if (r < 0)
+ goto err_dev;
+
+ /* if it's still not available, then the device is invalid */
+ sd->device = hashmap_get(sd->session->manager->devices, sp);
+ if (!sd->device) {
+ r = -ENODEV;
+ goto err_dev;
+ } else if (sd->device->seat != sd->session->seat) {
+ r = -EPERM;
+ goto err_dev;
+ }
+
+ udev_device_unref(p ? : dev);
+ return 0;
+
+err_dev:
+ udev_device_unref(p ? : dev);
+ return r;
+}
+
+int session_device_new(Session *s, const char *node, SessionDevice **out) {
+ SessionDevice *sd;
+ int r;
+
+ assert(s);
+ assert(node);
+ assert(out);
+
+ if (!s->seat)
+ return -EPERM;
+
+ sd = new0(SessionDevice, 1);
+ if (!sd)
+ return -ENOMEM;
+
+ sd->session = s;
+ sd->fd = -1;
+ sd->type = DEVICE_TYPE_UNKNOWN;
+
+ r = session_device_verify(sd, node);
+ if (r < 0)
+ goto error;
+
+ sd->node = strdup(node);
+ if (!sd->node) {
+ r = -ENOMEM;
+ goto error;
+ }
+
+ r = hashmap_put(s->devices, sd->node, sd);
+ if (r < 0) {
+ r = -ENOMEM;
+ goto error;
+ }
+
+ /* Open the device for the first time. We need a valid fd to pass back
+ * to the caller. If the session is not active, this _might_ immediately
+ * revoke access and thus invalidate the fd. But this is still needed
+ * to pass a valid fd back. */
+ sd->active = session_is_active(s);
+ sd->fd = session_device_open(sd, sd->active);
+ if (sd->fd < 0)
+ goto error;
+
+ LIST_PREPEND(SessionDevice, sd_by_device, sd->device->session_devices, sd);
+
+ *out = sd;
+ return 0;
+
+error:
+ if (sd->node) {
+ hashmap_remove(s->devices, sd->node);
+ free(sd->node);
+ }
+ free(sd);
+ return r;
+}
+
+void session_device_free(SessionDevice *sd) {
+ assert(sd);
+
+ session_device_stop(sd);
+ session_device_notify(sd, SESSION_DEVICE_RELEASE);
+ close_nointr_nofail(sd->fd);
+
+ LIST_REMOVE(SessionDevice, sd_by_device, sd->device->session_devices, sd);
+
+ hashmap_remove(sd->session->devices, sd->node);
+
+ free(sd->node);
+ free(sd);
+}
+
+void session_device_complete_pause(SessionDevice *sd) {
+ if (!sd->active)
+ return;
+
+ session_device_stop(sd);
+}
+
+void session_device_resume_all(Session *s) {
+ SessionDevice *sd;
+ Iterator i;
+ int r;
+
+ assert(s);
+
+ HASHMAP_FOREACH(sd, s->devices, i) {
+ if (!sd->active) {
+ r = session_device_start(sd);
+ if (!r)
+ session_device_notify(sd, SESSION_DEVICE_RESUME);
+ }
+ }
+}
+
+void session_device_pause_all(Session *s) {
+ SessionDevice *sd;
+ Iterator i;
+
+ assert(s);
+
+ HASHMAP_FOREACH(sd, s->devices, i) {
+ if (sd->active) {
+ session_device_stop(sd);
+ session_device_notify(sd, SESSION_DEVICE_PAUSE);
+ }
+ }
+}
diff --git a/src/login/logind-session-device.h b/src/login/logind-session-device.h
new file mode 100644
index 0000000..98e61e6
--- /dev/null
+++ b/src/login/logind-session-device.h
@@ -0,0 +1,57 @@
+/*-*- Mode: C; c-basic-offset: 8; indent-tabs-mode: nil -*-*/
+
+#pragma once
+
+/***
+ This file is part of systemd.
+
+ Copyright 2013 David Herrmann
+
+ systemd is free software; you can redistribute it and/or modify it
+ under the terms of the GNU Lesser General Public License as published by
+ the Free Software Foundation; either version 2.1 of the License, or
+ (at your option) any later version.
+
+ systemd is distributed in the hope that it will be useful, but
+ WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public License
+ along with systemd; If not, see <http://www.gnu.org/licenses/>.
+***/
+
+typedef struct SessionDevice SessionDevice;
+
+#include "list.h"
+#include "util.h"
+#include "logind.h"
+#include "logind-device.h"
+#include "logind-seat.h"
+#include "logind-session.h"
+
+enum DeviceType {
+ DEVICE_TYPE_UNKNOWN,
+ DEVICE_TYPE_FBDEV,
+ DEVICE_TYPE_DRM,
+ DEVICE_TYPE_EVDEV,
+};
+
+struct SessionDevice {
+ Session *session;
+ Device *device;
+
+ char *node;
+ int fd;
+ bool active;
+ enum DeviceType type;
+
+ LIST_FIELDS(struct SessionDevice, sd_by_device);
+};
+
+int session_device_new(Session *s, const char *node, SessionDevice **out);
+void session_device_free(SessionDevice *sd);
+void session_device_complete_pause(SessionDevice *sd);
+
+void session_device_resume_all(Session *s);
+void session_device_pause_all(Session *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index a11804a..69a8ad7 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -52,9 +52,17 @@ Session* session_new(Manager *m, const char *id) {
return NULL;
}

+ s->devices = hashmap_new(string_hash_func, string_compare_func);
+ if (!s->devices) {
+ free(s->state_file);
+ free(s);
+ return NULL;
+ }
+
s->id = path_get_file_name(s->state_file);

if (hashmap_put(m->sessions, s->id, s) < 0) {
+ hashmap_free(s->devices);
free(s->state_file);
free(s);
return NULL;
@@ -67,6 +75,8 @@ Session* session_new(Manager *m, const char *id) {
}

void session_free(Session *s) {
+ SessionDevice *sd;
+
assert(s);

if (s->in_gc_queue)
@@ -74,6 +84,11 @@ void session_free(Session *s) {

session_drop_controller(s);

+ while ((sd = hashmap_first(s->devices)))
+ session_device_free(sd);
+
+ hashmap_free(s->devices);
+
if (s->user) {
LIST_REMOVE(Session, sessions_by_user, s->user->sessions, s);

@@ -619,6 +634,7 @@ int session_stop(Session *s) {

int session_finalize(Session *s) {
int r = 0;
+ SessionDevice *sd;

assert(s);

@@ -634,6 +650,10 @@ int session_finalize(Session *s) {
"MESSAGE=Removed session %s.", s->id,
NULL);

+ /* Kill session devices */
+ while ((sd = hashmap_first(s->devices)))
+ session_device_free(sd);
+
/* Remove X11 symlink */
session_unlink_x11_socket(s);

@@ -957,6 +977,8 @@ int session_set_controller(Session *s, const char *sender, bool force) {
}

void session_drop_controller(Session *s) {
+ SessionDevice *sd;
+
assert(s);

if (!s->controller)
@@ -965,6 +987,11 @@ void session_drop_controller(Session *s) {
manager_drop_busname(s->manager, s->controller);
free(s->controller);
s->controller = NULL;
+
+ /* Drop all devices as they're now unused. Do that after the controller
+ * is released to avoid sending out useles dbus signals. */
+ while ((sd = hashmap_first(s->devices)))
+ session_device_free(sd);
}

static const char* const session_state_table[_SESSION_STATE_MAX] = {
diff --git a/src/login/logind-session.h b/src/login/logind-session.h
index 411a1b1..6737608 100644
--- a/src/login/logind-session.h
+++ b/src/login/logind-session.h
@@ -28,6 +28,7 @@ typedef enum KillWho KillWho;
#include "util.h"
#include "logind.h"
#include "logind-seat.h"
+#include "logind-session-device.h"
#include "logind-user.h"

typedef enum SessionState {
@@ -106,6 +107,7 @@ struct Session {
DBusMessage *create_message;

char *controller;
+ Hashmap *devices;

LIST_FIELDS(Session, sessions_by_user);
LIST_FIELDS(Session, sessions_by_seat);
--
1.8.3.4
David Herrmann
2013-08-25 12:46:11 UTC
Permalink
The seat->vtconsole member always points to the default seat seat0. Even
if VTs are disabled, it's used as default seat. Therefore, rename it to
seat0 to correctly state what it is.

This also changes the seat files in /run from IS_VTCONSOLE to IS_SEAT0. It
wasn't used by any code, yet, so this seems fine.

While we are at it, we also remove every "if (s->vtconsole)" as this
pointer is always valid!
---
src/login/logind-dbus.c | 8 ++++----
src/login/logind-seat.c | 14 +++++++-------
src/login/logind-seat.h | 2 +-
src/login/logind-session.c | 2 +-
src/login/logind.c | 8 ++++----
src/login/logind.h | 2 +-
6 files changed, 18 insertions(+), 18 deletions(-)

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index a703e59..8940aeb 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -404,8 +404,8 @@ static int bus_manager_create_session(Manager *m, DBusMessage *message) {
int v;

if (!seat)
- seat = m->vtconsole;
- else if (seat != m->vtconsole)
+ seat = m->seat0;
+ else if (seat != m->seat0)
return -EINVAL;

v = vtnr_from_tty(tty);
@@ -420,8 +420,8 @@ static int bus_manager_create_session(Manager *m, DBusMessage *message) {
} else if (tty_is_console(tty)) {

if (!seat)
- seat = m->vtconsole;
- else if (seat != m->vtconsole)
+ seat = m->seat0;
+ else if (seat != m->seat0)
return -EINVAL;

if (vtnr != 0)
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index dcaf0ac..88fd724 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -105,11 +105,11 @@ int seat_save(Seat *s) {

fprintf(f,
"# This is private data. Do not parse.\n"
- "IS_VTCONSOLE=%i\n"
+ "IS_SEAT0=%i\n"
"CAN_MULTI_SESSION=%i\n"
"CAN_TTY=%i\n"
"CAN_GRAPHICAL=%i\n",
- seat_is_vtconsole(s),
+ seat_is_seat0(s),
seat_can_multi_session(s),
seat_can_tty(s),
seat_can_graphical(s));
@@ -429,16 +429,16 @@ int seat_attach_session(Seat *s, Session *session) {
return 0;
}

-bool seat_is_vtconsole(Seat *s) {
+bool seat_is_seat0(Seat *s) {
assert(s);

- return s->manager->vtconsole == s;
+ return s->manager->seat0 == s;
}

bool seat_can_multi_session(Seat *s) {
assert(s);

- if (!seat_is_vtconsole(s))
+ if (!seat_is_seat0(s))
return false;

/* If we can't watch which VT is in the foreground, we don't
@@ -450,7 +450,7 @@ bool seat_can_multi_session(Seat *s) {
bool seat_can_tty(Seat *s) {
assert(s);

- return seat_is_vtconsole(s);
+ return seat_is_seat0(s);
}

bool seat_has_master_device(Seat *s) {
@@ -508,7 +508,7 @@ int seat_check_gc(Seat *s, bool drop_not_started) {
if (drop_not_started && !s->started)
return 0;

- if (seat_is_vtconsole(s))
+ if (seat_is_seat0(s))
return 1;

return seat_has_master_device(s);
diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index bd5390f..47fe89a 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -60,7 +60,7 @@ int seat_preallocate_vts(Seat *s);

int seat_attach_session(Seat *s, Session *session);

-bool seat_is_vtconsole(Seat *s);
+bool seat_is_seat0(Seat *s);
bool seat_can_multi_session(Seat *s);
bool seat_can_tty(Seat *s);
bool seat_has_master_device(Seat *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 69a8ad7..ae91650 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -386,7 +386,7 @@ int session_activate(Session *s) {
if (s->seat->active == s)
return 0;

- assert(seat_is_vtconsole(s->seat));
+ assert(seat_is_seat0(s->seat));

return chvt(s->vtnr);
}
diff --git a/src/login/logind.c b/src/login/logind.c
index ad3ab81..d18bb08 100644
--- a/src/login/logind.c
+++ b/src/login/logind.c
@@ -852,7 +852,7 @@ int manager_dispatch_vcsa_udev(Manager *m) {
* VTs, to make sure our auto VTs never go away. */

if (name && startswith(name, "vcsa") && streq_ptr(udev_device_get_action(d), "remove"))
- r = seat_preallocate_vts(m->vtconsole);
+ r = seat_preallocate_vts(m->seat0);

udev_device_unref(d);

@@ -877,9 +877,9 @@ int manager_dispatch_button_udev(Manager *m) {

int manager_dispatch_console(Manager *m) {
assert(m);
+ assert(m->seat0);

- if (m->vtconsole)
- seat_read_active_vt(m->vtconsole);
+ seat_read_active_vt(m->seat0);

return 0;
}
@@ -1537,7 +1537,7 @@ int manager_startup(Manager *m) {
return r;

/* Instantiate magic seat 0 */
- r = manager_add_seat(m, "seat0", &m->vtconsole);
+ r = manager_add_seat(m, "seat0", &m->seat0);
if (r < 0)
return r;

diff --git a/src/login/logind.h b/src/login/logind.h
index a76936d..9e6296c 100644
--- a/src/login/logind.h
+++ b/src/login/logind.h
@@ -74,7 +74,7 @@ struct Manager {
unsigned reserve_vt;
int reserve_vt_fd;

- Seat *vtconsole;
+ Seat *seat0;

char **kill_only_users, **kill_exclude_users;
bool kill_user_processes;
--
1.8.3.4
David Herrmann
2013-08-25 12:46:12 UTC
Permalink
A seat provides text-logins if it has VTs. This is always limited to seat0
so the seat_is_seat0() check is correct. However, if VTs are disabled, no
seat provides text-logins so we also need to check for the console-fd.

This was previously:
return seat_is_vtconsole();
It looked right, but was functionally equivalent to seat_is_seat0(). The
rename of this helper made it more obvious that it is missing the VT test.
---
src/login/logind-seat.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index 88fd724..4c2c424 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -450,7 +450,7 @@ bool seat_can_multi_session(Seat *s) {
bool seat_can_tty(Seat *s) {
assert(s);

- return seat_is_seat0(s);
+ return seat_is_seat0(s) && s->manager->console_active_fd >= 0;
}

bool seat_has_master_device(Seat *s) {
--
1.8.3.4
David Herrmann
2013-08-25 12:46:13 UTC
Permalink
VT numbers start with 1. If a session has vtnr == 0, we must not assume it
is running on a VT.
Note that this could trigger the assert() below as CreateSession() sets
vtnr to 0, not <0.
---
src/login/logind-session.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index ae91650..50ba6b8 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -377,7 +377,7 @@ int session_activate(Session *s) {
assert(s);
assert(s->user);

- if (s->vtnr < 0)
+ if (s->vtnr <= 0)
return -ENOTSUP;

if (!s->seat)
--
1.8.3.4
David Herrmann
2013-08-25 12:46:14 UTC
Permalink
We currently use seat_can_multi_session() to test for two things:
* whether the seat can handle session-switching
* whether the seat has VTs

As both are currently logically equivalent, we didn't care. However, we
want to allow session-switching on seats without VTs, so split this helper
into:
* seat_can_multi_session(): whether session-switching is supported
* seat_has_vts(): whether the seat has VTs

Note that only one seat on a system can have VTs. There is only one set of
them. We automatically assign them to seat0 as usual.

With this patch in place, we can easily add new session-switching/tracking
methods without breaking any VT code as it is now protected by has_vts(),
no longer by can_multi_session().
---
src/login/logind-dbus.c | 2 +-
src/login/logind-seat.c | 32 ++++++++++++++------------------
src/login/logind-seat.h | 1 +
src/login/logind-session.c | 6 +++---
4 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/src/login/logind-dbus.c b/src/login/logind-dbus.c
index 8940aeb..5e42c0a 100644
--- a/src/login/logind-dbus.c
+++ b/src/login/logind-dbus.c
@@ -429,7 +429,7 @@ static int bus_manager_create_session(Manager *m, DBusMessage *message) {
}

if (seat) {
- if (seat_can_multi_session(seat)) {
+ if (seat_has_vts(seat)) {
if (vtnr > 63)
return -EINVAL;
} else {
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index 4c2c424..f88738a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -201,7 +201,7 @@ int seat_preallocate_vts(Seat *s) {
if (s->manager->n_autovts <= 0)
return 0;

- if (!seat_can_multi_session(s))
+ if (!seat_has_vts(s))
return 0;

for (i = 1; i <= s->manager->n_autovts; i++) {
@@ -282,7 +282,7 @@ int seat_active_vt_changed(Seat *s, int vtnr) {
assert(s);
assert(vtnr >= 1);

- if (!seat_can_multi_session(s))
+ if (!seat_has_vts(s))
return -EINVAL;

log_debug("VT changed to %i", vtnr);
@@ -306,7 +306,7 @@ int seat_read_active_vt(Seat *s) {

assert(s);

- if (!seat_can_multi_session(s))
+ if (!seat_has_vts(s))
return 0;

lseek(s->manager->console_active_fd, SEEK_SET, 0);
@@ -417,18 +417,20 @@ int seat_attach_session(Seat *s, Session *session) {

seat_send_changed(s, "Sessions\0");

- /* Note that even if a seat is not multi-session capable it
- * still might have multiple sessions on it since old, dead
- * sessions might continue to be tracked until all their
- * processes are gone. The most recently added session
- * (i.e. the first in s->sessions) is the one that matters. */
-
- if (!seat_can_multi_session(s))
+ /* On seats with VTs, the VT logic defines which session is active. On
+ * seats without VTs, we automatically activate the first session. */
+ if (!seat_has_vts(s) && !s->active)
seat_set_active(s, session);

return 0;
}

+bool seat_has_vts(Seat *s) {
+ assert(s);
+
+ return seat_is_seat0(s) && s->manager->console_active_fd >= 0;
+}
+
bool seat_is_seat0(Seat *s) {
assert(s);

@@ -438,19 +440,13 @@ bool seat_is_seat0(Seat *s) {
bool seat_can_multi_session(Seat *s) {
assert(s);

- if (!seat_is_seat0(s))
- return false;
-
- /* If we can't watch which VT is in the foreground, we don't
- * support VT switching */
-
- return s->manager->console_active_fd >= 0;
+ return seat_has_vts(s);
}

bool seat_can_tty(Seat *s) {
assert(s);

- return seat_is_seat0(s) && s->manager->console_active_fd >= 0;
+ return seat_has_vts(s);
}

bool seat_has_master_device(Seat *s) {
diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index 47fe89a..d3438b8 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -60,6 +60,7 @@ int seat_preallocate_vts(Seat *s);

int seat_attach_session(Seat *s, Session *session);

+bool seat_has_vts(Seat *s);
bool seat_is_seat0(Seat *s);
bool seat_can_multi_session(Seat *s);
bool seat_can_tty(Seat *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index 50ba6b8..e7fe0f6 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -205,7 +205,7 @@ int session_save(Session *s) {
if (s->service)
fprintf(f, "SERVICE=%s\n", s->service);

- if (s->seat && seat_can_multi_session(s->seat))
+ if (s->seat && seat_has_vts(s->seat))
fprintf(f, "VTNR=%i\n", s->vtnr);

if (s->leader > 0)
@@ -315,7 +315,7 @@ int session_load(Session *s) {
seat_attach_session(o, s);
}

- if (vtnr && s->seat && seat_can_multi_session(s->seat)) {
+ if (vtnr && s->seat && seat_has_vts(s->seat)) {
int v;

k = safe_atoi(vtnr, &v);
@@ -386,7 +386,7 @@ int session_activate(Session *s) {
if (s->seat->active == s)
return 0;

- assert(seat_is_seat0(s->seat));
+ assert(seat_has_vts(s->seat));

return chvt(s->vtnr);
}
--
1.8.3.4
David Herrmann
2013-08-25 12:46:15 UTC
Permalink
This enables the multi-session capability for seats that don't have VTs.
For legacy seats with VTs, everything stays the same. However, all other
seats now also get the multi-session capability.

The only feature that was missing was session-switching. As logind can
force a session-switch and signal that via the "Active" property, we only
need a way to allow synchronized/delayed session switches. Compositors
need to cleanup some devices before acknowledging the session switch.
Therefore, we use the session-devices to give compositors a chance to
block a session-switch until they cleaned everything up.

If you activate a session on a seat without VTs, we send a PauseDevice
signal to the active session for every active device. Only once the
session acknowledged all these with a PauseDeviceComplete() call, we
perform the final session switch.

One important note is that delayed session-switching is meant for
backwards compatibility. New compositors or other sessions should really
try to deal correctly with forced session switches! They only need to
handle EACCES/EPERM from syscalls and treat them as "PauseDevice" signal.

Following logind patches will add a timeout to session-switches which
forces the switch if the active session does not react in a timely
fashion. Moreover, explicit ForceActivate() calls might also be supported.
Hence, sessions must not crash if their devices get paused.
---
src/login/logind-seat.c | 15 +++++++++++++++
src/login/logind-seat.h | 2 ++
src/login/logind-session-device.c | 28 ++++++++++++++++++++++++++++
src/login/logind-session-device.h | 1 +
src/login/logind-session.c | 31 ++++++++++++++++++++++++++-----
5 files changed, 72 insertions(+), 5 deletions(-)

diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index f88738a..4a4d40a 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -425,6 +425,21 @@ int seat_attach_session(Seat *s, Session *session) {
return 0;
}

+void seat_complete_switch(Seat *s) {
+ Session *session;
+
+ assert(s);
+
+ /* if no session-switch is pending or if it got canceled, do nothing */
+ if (!s->pending_switch)
+ return;
+
+ session = s->pending_switch;
+ s->pending_switch = NULL;
+
+ seat_set_active(s, session);
+}
+
bool seat_has_vts(Seat *s) {
assert(s);

diff --git a/src/login/logind-seat.h b/src/login/logind-seat.h
index d3438b8..be6db6e 100644
--- a/src/login/logind-seat.h
+++ b/src/login/logind-seat.h
@@ -38,6 +38,7 @@ struct Seat {
LIST_HEAD(Device, devices);

Session *active;
+ Session *pending_switch;
LIST_HEAD(Session, sessions);

bool in_gc_queue:1;
@@ -59,6 +60,7 @@ int seat_read_active_vt(Seat *s);
int seat_preallocate_vts(Seat *s);

int seat_attach_session(Seat *s, Session *session);
+void seat_complete_switch(Seat *s);

bool seat_has_vts(Seat *s);
bool seat_is_seat0(Seat *s);
diff --git a/src/login/logind-session-device.c b/src/login/logind-session-device.c
index 7c6b07f..7123955 100644
--- a/src/login/logind-session-device.c
+++ b/src/login/logind-session-device.c
@@ -452,10 +452,21 @@ void session_device_free(SessionDevice *sd) {
}

void session_device_complete_pause(SessionDevice *sd) {
+ SessionDevice *iter;
+ Iterator i;
+
if (!sd->active)
return;

session_device_stop(sd);
+
+ /* if not all devices are paused, wait for further completion events */
+ HASHMAP_FOREACH(iter, sd->session->devices, i)
+ if (iter->active)
+ return;
+
+ /* complete any pending session switch */
+ seat_complete_switch(sd->session->seat);
}

void session_device_resume_all(Session *s) {
@@ -487,3 +498,20 @@ void session_device_pause_all(Session *s) {
}
}
}
+
+unsigned int session_device_try_pause_all(Session *s) {
+ SessionDevice *sd;
+ Iterator i;
+ unsigned int num_pending = 0;
+
+ assert(s);
+
+ HASHMAP_FOREACH(sd, s->devices, i) {
+ if (sd->active) {
+ session_device_notify(sd, SESSION_DEVICE_TRY_PAUSE);
+ ++num_pending;
+ }
+ }
+
+ return num_pending;
+}
diff --git a/src/login/logind-session-device.h b/src/login/logind-session-device.h
index 98e61e6..61732bc 100644
--- a/src/login/logind-session-device.h
+++ b/src/login/logind-session-device.h
@@ -55,3 +55,4 @@ void session_device_complete_pause(SessionDevice *sd);

void session_device_resume_all(Session *s);
void session_device_pause_all(Session *s);
+unsigned int session_device_try_pause_all(Session *s);
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index e7fe0f6..61366c1 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -99,6 +99,8 @@ void session_free(Session *s) {
if (s->seat) {
if (s->seat->active == s)
s->seat->active = NULL;
+ if (s->seat->pending_switch == s)
+ s->seat->pending_switch = NULL;

LIST_REMOVE(Session, sessions_by_seat, s->seat->sessions, s);
}
@@ -374,21 +376,40 @@ int session_load(Session *s) {
}

int session_activate(Session *s) {
+ unsigned int num_pending;
+
assert(s);
assert(s->user);

- if (s->vtnr <= 0)
- return -ENOTSUP;
-
if (!s->seat)
return -ENOTSUP;

if (s->seat->active == s)
return 0;

- assert(seat_has_vts(s->seat));
+ /* on seats with VTs, we let VTs manage session-switching */
+ if (seat_has_vts(s->seat)) {
+ if (s->vtnr <= 0)
+ return -ENOTSUP;
+
+ return chvt(s->vtnr);
+ }

- return chvt(s->vtnr);
+ /* On seats without VTs, we implement session-switching in logind. We
+ * try to pause all session-devices and wait until the session
+ * controller acknowledged them. Once all devices are asleep, we simply
+ * switch the active session and be done.
+ * We save the session we want to switch to in seat->pending_switch and
+ * seat_complete_switch() will perform the final switch. */
+
+ s->seat->pending_switch = s;
+
+ /* if no devices are running, immediately perform the session switch */
+ num_pending = session_device_try_pause_all(s);
+ if (!num_pending)
+ seat_complete_switch(s->seat);
+
+ return 0;
}

static int session_link_x11_socket(Session *s) {
--
1.8.3.4
Tom Gundersen
2013-08-25 15:17:33 UTC
Permalink
Hi David,
Post by David Herrmann
One important note is that delayed session-switching is meant for
backwards compatibility. New compositors or other sessions should really
try to deal correctly with forced session switches! They only need to
handle EACCES/EPERM from syscalls and treat them as "PauseDevice" signal.
Following logind patches will add a timeout to session-switches which
forces the switch if the active session does not react in a timely
fashion. Moreover, explicit ForceActivate() calls might also be supported.
Hence, sessions must not crash if their devices get paused.
I don't quite get the backwards compatibility argument. In order to
make use of this, compositors would anyway have to be ported to the
logind interface, so why not just require these compositors to handle
EACCESS/EPERM as you describe and avoid the backwards compatibility
stuff? Do you have reason to believe that handling EACCESS would be
significantly harder than just porting to the PauseDevice interface?
Or is there something else I'm missing?

Cheers,

Tom
David Herrmann
2013-08-25 15:27:11 UTC
Permalink
Hi
Post by Tom Gundersen
Hi David,
Post by David Herrmann
One important note is that delayed session-switching is meant for
backwards compatibility. New compositors or other sessions should really
try to deal correctly with forced session switches! They only need to
handle EACCES/EPERM from syscalls and treat them as "PauseDevice" signal.
Following logind patches will add a timeout to session-switches which
forces the switch if the active session does not react in a timely
fashion. Moreover, explicit ForceActivate() calls might also be supported.
Hence, sessions must not crash if their devices get paused.
[...]
Post by Tom Gundersen
Do you have reason to believe that handling EACCESS would be
significantly harder than just porting to the PauseDevice interface?
[...]

Yes.

For the XServer device handling is done in drivers. There are many of
xf86-video-* drivers and I have no intention to fix all of them. Note
that device-enumeration (udev etc) is done in the xserver core. This
could be fixed more easily.

But as said in the introduction-text, this is *only* for
compatibility. And the longer I think about it, I get more and more
convinced to drop it all-together. It's handy to test it and play
around with it, but maybe the final revision would be better off
without it. If anyone wants to run an xserver with it, they ought to
use a driver that can deal with asynchronous session-switches.

Cheers
David
Tom Gundersen
2013-08-26 04:37:03 UTC
Permalink
Post by David Herrmann
logind itselfs takes care of revoking device access for inactive sessions
(synchronized with session-switches!). It also tries to resume every device
when a session is activated. But session-devices must not be used to watch
session state! A compositor has to use the PropertiesChanged() signal plus the
"Active" property of sessions for that! Session-devices do not replace this! On
sessions with VTs, this is obviously replaced by the VT_SETMODE interface as
usual.
So essentially, all clients have to keep implementing the old
VT_SETMODE interface in addition to the new logind one?

Would it be possible to ship a wrapper around VT_SETMODE and friends
so that new clients could only implement the logind interface, and
logind would deal with the VT layer internally (or rather, I suppose,
via some shim session daemon) when necessary?

-t
David Herrmann
2013-08-26 16:00:56 UTC
Permalink
Hi
Post by Tom Gundersen
Post by David Herrmann
logind itselfs takes care of revoking device access for inactive sessions
(synchronized with session-switches!). It also tries to resume every device
when a session is activated. But session-devices must not be used to watch
session state! A compositor has to use the PropertiesChanged() signal plus the
"Active" property of sessions for that! Session-devices do not replace this! On
sessions with VTs, this is obviously replaced by the VT_SETMODE interface as
usual.
So essentially, all clients have to keep implementing the old
VT_SETMODE interface in addition to the new logind one?
Would it be possible to ship a wrapper around VT_SETMODE and friends
so that new clients could only implement the logind interface, and
logind would deal with the VT layer internally (or rather, I suppose,
via some shim session daemon) when necessary?
It's an orthogonal problem. And you don't need to handle it at all.
The default for VT_SETMODE is a forced VT switch. So as long as no
process in a session opens the VT and modifies VT_SETMODE, VT switch
will "Just Work". And logind is notified via
/sys/class/tty/tty0/active

Thanks for looking over it, btw!
David
Tom Gundersen
2013-08-27 08:22:04 UTC
Permalink
Post by David Herrmann
Hi
Post by Tom Gundersen
Post by David Herrmann
logind itselfs takes care of revoking device access for inactive sessions
(synchronized with session-switches!). It also tries to resume every device
when a session is activated. But session-devices must not be used to watch
session state! A compositor has to use the PropertiesChanged() signal plus the
"Active" property of sessions for that! Session-devices do not replace this! On
sessions with VTs, this is obviously replaced by the VT_SETMODE interface as
usual.
So essentially, all clients have to keep implementing the old
VT_SETMODE interface in addition to the new logind one?
Would it be possible to ship a wrapper around VT_SETMODE and friends
so that new clients could only implement the logind interface, and
logind would deal with the VT layer internally (or rather, I suppose,
via some shim session daemon) when necessary?
It's an orthogonal problem. And you don't need to handle it at all.
The default for VT_SETMODE is a forced VT switch. So as long as no
process in a session opens the VT and modifies VT_SETMODE, VT switch
will "Just Work". And logind is notified via
/sys/class/tty/tty0/active
Perfect. Thanks for the explanation.

Tom

Continue reading on narkive:
Loading...