From 1dba5a0b194653b0effb01a918bd7338b0c6bcb9 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 21 Jan 2015 09:31:39 +0100 Subject: [PATCH 1/5] linux: Add linux_parse_vt_settings and linux_get_keeptty helpers systemd-logind integration does not work when starting X on a new tty, as that detaches X from the current session and after hat systemd-logind revokes all rights on any already open fds and refuses to open new fds for X. This means that currently e.g. "startx -- vt7" breaks, and breaks badly, requiring ssh access to the system to kill X. The fix for this is easy, we must not use systemd-logind integration when not using KeepTty, or iow we may only use systemd-logind integration together with KeepTty. But the final KeepTty value is not known until the code to chose which vtno to run on has been called, which currently happens after intializing systemd-logind. This commit is step 1 in fixing the "startx -- vt7" breakage, it factors out the linux xf86OpenConsole bits which set xf86Info.vtno and keepTty so that these can be called earlier. Calling this earlier is safe as this code has no side effects other than setting xf86Info.vtno and keepTty. Note this basically only moves a large chunk of xf86OpenConsole() into linux_parse_vt_settings() without changing a single line of it, this is hard to see in the diff because the identation level has changed. Signed-off-by: Hans de Goede Reviewed-by: Peter Hutterer --- hw/xfree86/os-support/linux/linux.h | 32 ++++++ hw/xfree86/os-support/linux/lnx_init.c | 136 +++++++++++++++---------- 2 files changed, 112 insertions(+), 56 deletions(-) create mode 100644 hw/xfree86/os-support/linux/linux.h diff --git a/hw/xfree86/os-support/linux/linux.h b/hw/xfree86/os-support/linux/linux.h new file mode 100644 index 000000000..8cb8e3d73 --- /dev/null +++ b/hw/xfree86/os-support/linux/linux.h @@ -0,0 +1,32 @@ +/* + * Copyright © 2015 Red Hat, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + * + * Author: Hans de Goede + */ + +#ifndef XF86_LINUX_H +#define XF86_LINUX_H + +void linux_parse_vt_settings(void); +int linux_get_keeptty(void); + +#endif diff --git a/hw/xfree86/os-support/linux/lnx_init.c b/hw/xfree86/os-support/linux/lnx_init.c index a0e6782b0..7032825e1 100644 --- a/hw/xfree86/os-support/linux/lnx_init.c +++ b/hw/xfree86/os-support/linux/lnx_init.c @@ -31,6 +31,7 @@ #include #include "compiler.h" +#include "linux.h" #include "xf86.h" #include "xf86Priv.h" @@ -80,71 +81,94 @@ switch_to(int vt, const char *from) #pragma GCC diagnostic ignored "-Wformat-nonliteral" void -xf86OpenConsole(void) +linux_parse_vt_settings(void) { int i, fd = -1, ret, current_vt = -1; - struct vt_mode VT; struct vt_stat vts; struct stat st; MessageType from = X_PROBED; const char *tty0[] = { "/dev/tty0", "/dev/vc/0", NULL }; + + /* Only do this once */ + static int vt_settings_parsed = 0; + + if (vt_settings_parsed) + return; + + /* + * setup the virtual terminal manager + */ + if (xf86Info.vtno != -1) { + from = X_CMDLINE; + } + else { + + i = 0; + while (tty0[i] != NULL) { + if ((fd = open(tty0[i], O_WRONLY, 0)) >= 0) + break; + i++; + } + + if (fd < 0) + FatalError("parse_vt_settings: Cannot open /dev/tty0 (%s)\n", + strerror(errno)); + + if (xf86Info.ShareVTs) { + SYSCALL(ret = ioctl(fd, VT_GETSTATE, &vts)); + if (ret < 0) + FatalError("parse_vt_settings: Cannot find the current" + " VT (%s)\n", strerror(errno)); + xf86Info.vtno = vts.v_active; + } + else { + SYSCALL(ret = ioctl(fd, VT_OPENQRY, &xf86Info.vtno)); + if (ret < 0) + FatalError("parse_vt_settings: Cannot find a free VT: " + "%s\n", strerror(errno)); + if (xf86Info.vtno == -1) + FatalError("parse_vt_settings: Cannot find a free VT\n"); + } + close(fd); + } + + xf86Msg(from, "using VT number %d\n\n", xf86Info.vtno); + + /* Some of stdin / stdout / stderr maybe redirected to a file */ + for (i = STDIN_FILENO; i <= STDERR_FILENO; i++) { + ret = fstat(i, &st); + if (ret == 0 && S_ISCHR(st.st_mode) && major(st.st_rdev) == 4) { + current_vt = minor(st.st_rdev); + break; + } + } + + if (!KeepTty && current_vt == xf86Info.vtno) { + xf86Msg(X_PROBED, + "controlling tty is VT number %d, auto-enabling KeepTty\n", + current_vt); + KeepTty = TRUE; + } + + vt_settings_parsed = 1; +} + +int +linux_get_keeptty(void) +{ + return KeepTty; +} + +void +xf86OpenConsole(void) +{ + int i, ret; + struct vt_stat vts; + struct vt_mode VT; const char *vcs[] = { "/dev/vc/%d", "/dev/tty%d", NULL }; if (serverGeneration == 1) { - /* - * setup the virtual terminal manager - */ - if (xf86Info.vtno != -1) { - from = X_CMDLINE; - } - else { - - i = 0; - while (tty0[i] != NULL) { - if ((fd = open(tty0[i], O_WRONLY, 0)) >= 0) - break; - i++; - } - - if (fd < 0) - FatalError("xf86OpenConsole: Cannot open /dev/tty0 (%s)\n", - strerror(errno)); - - if (xf86Info.ShareVTs) { - SYSCALL(ret = ioctl(fd, VT_GETSTATE, &vts)); - if (ret < 0) - FatalError("xf86OpenConsole: Cannot find the current" - " VT (%s)\n", strerror(errno)); - xf86Info.vtno = vts.v_active; - } - else { - SYSCALL(ret = ioctl(fd, VT_OPENQRY, &xf86Info.vtno)); - if (ret < 0) - FatalError("xf86OpenConsole: Cannot find a free VT: " - "%s\n", strerror(errno)); - if (xf86Info.vtno == -1) - FatalError("xf86OpenConsole: Cannot find a free VT\n"); - } - close(fd); - } - - xf86Msg(from, "using VT number %d\n\n", xf86Info.vtno); - - /* Some of stdin / stdout / stderr maybe redirected to a file */ - for (i = STDIN_FILENO; i <= STDERR_FILENO; i++) { - ret = fstat(i, &st); - if (ret == 0 && S_ISCHR(st.st_mode) && major(st.st_rdev) == 4) { - current_vt = minor(st.st_rdev); - break; - } - } - - if (!KeepTty && current_vt == xf86Info.vtno) { - xf86Msg(X_PROBED, - "controlling tty is VT number %d, auto-enabling KeepTty\n", - current_vt); - KeepTty = TRUE; - } + linux_parse_vt_settings(); if (!KeepTty) { pid_t ppid = getppid(); From 81bcada14e339fe2a2fb3f3a040566d94dc20bad Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 13 May 2015 13:17:09 +0200 Subject: [PATCH 2/5] linux: Add a may_fail paramter to linux_parse_vt_settings linux_parse_vt_settings() was split out of xf86OpenConsole so that it can be called earlier during systemd-logind init, but it is possible to run the xserver in such a way that xf86OpenConsole() is never used. The FatalError calls in linux_parse_vt_settings() may stop the Xorg xserver from working when e.g. no /dev/tty0 is present in such a setup. This commit adds a may_fail parameter to linux_parse_vt_settings() which can be used to make linux_parse_vt_settings() fail silenty with an error return in this case, rather then calling FatalError(). Signed-off-by: Hans de Goede Reviewed-by: Peter Hutterer --- hw/xfree86/os-support/linux/linux.h | 2 +- hw/xfree86/os-support/linux/lnx_init.c | 29 +++++++++++++++++++------- 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/hw/xfree86/os-support/linux/linux.h b/hw/xfree86/os-support/linux/linux.h index 8cb8e3d73..83506fd38 100644 --- a/hw/xfree86/os-support/linux/linux.h +++ b/hw/xfree86/os-support/linux/linux.h @@ -26,7 +26,7 @@ #ifndef XF86_LINUX_H #define XF86_LINUX_H -void linux_parse_vt_settings(void); +int linux_parse_vt_settings(int may_fail); int linux_get_keeptty(void); #endif diff --git a/hw/xfree86/os-support/linux/lnx_init.c b/hw/xfree86/os-support/linux/lnx_init.c index 7032825e1..ec06a059e 100644 --- a/hw/xfree86/os-support/linux/lnx_init.c +++ b/hw/xfree86/os-support/linux/lnx_init.c @@ -80,8 +80,8 @@ switch_to(int vt, const char *from) #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wformat-nonliteral" -void -linux_parse_vt_settings(void) +int +linux_parse_vt_settings(int may_fail) { int i, fd = -1, ret, current_vt = -1; struct vt_stat vts; @@ -93,7 +93,7 @@ linux_parse_vt_settings(void) static int vt_settings_parsed = 0; if (vt_settings_parsed) - return; + return 1; /* * setup the virtual terminal manager @@ -110,24 +110,36 @@ linux_parse_vt_settings(void) i++; } - if (fd < 0) + if (fd < 0) { + if (may_fail) + return 0; FatalError("parse_vt_settings: Cannot open /dev/tty0 (%s)\n", strerror(errno)); + } if (xf86Info.ShareVTs) { SYSCALL(ret = ioctl(fd, VT_GETSTATE, &vts)); - if (ret < 0) + if (ret < 0) { + if (may_fail) + return 0; FatalError("parse_vt_settings: Cannot find the current" " VT (%s)\n", strerror(errno)); + } xf86Info.vtno = vts.v_active; } else { SYSCALL(ret = ioctl(fd, VT_OPENQRY, &xf86Info.vtno)); - if (ret < 0) + if (ret < 0) { + if (may_fail) + return 0; FatalError("parse_vt_settings: Cannot find a free VT: " "%s\n", strerror(errno)); - if (xf86Info.vtno == -1) + } + if (xf86Info.vtno == -1) { + if (may_fail) + return 0; FatalError("parse_vt_settings: Cannot find a free VT\n"); + } } close(fd); } @@ -151,6 +163,7 @@ linux_parse_vt_settings(void) } vt_settings_parsed = 1; + return 1; } int @@ -168,7 +181,7 @@ xf86OpenConsole(void) const char *vcs[] = { "/dev/vc/%d", "/dev/tty%d", NULL }; if (serverGeneration == 1) { - linux_parse_vt_settings(); + linux_parse_vt_settings(FALSE); if (!KeepTty) { pid_t ppid = getppid(); From c88a325899381d1133e63e6c33c29db079e20a87 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 21 Jan 2015 10:13:20 +0100 Subject: [PATCH 3/5] systemd-logind: Only use systemd-logind integration together with keeptty systemd-logind integration does not work when starting X on a new tty, as that detaches X from the current session and after hat systemd-logind revokes all rights any already open fds and refuses to open new fds for X. This means that currently e.g. "startx -- vt7" breaks, and breaks badly, requiring ssh access to the system to kill X. The fix for this is easy, we must not use systemd-logind integration when not using KeepTty, or iow we may only use systemd-logind integration together with KeepTty. Signed-off-by: Hans de Goede Reviewed-by: Peter Hutterer Tested-by: Aaron Plattner --- hw/xfree86/man/Xorg.man | 6 +++--- hw/xfree86/os-support/linux/systemd-logind.c | 8 ++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/hw/xfree86/man/Xorg.man b/hw/xfree86/man/Xorg.man index 3ff6aef89..ddf135866 100644 --- a/hw/xfree86/man/Xorg.man +++ b/hw/xfree86/man/Xorg.man @@ -271,9 +271,9 @@ is ignored if is anything other than \(oqPCI\(cq. .TP 8 .B \-keeptty -Prevent the server from detaching its initial controlling terminal. -This option is only useful when debugging the server. Not all platforms -support (or can use) this option. +Prevent the server from detaching its initial controlling terminal. If you +want to use systemd-logind integration you must specify this option. +Not all platforms support (or can use) this option. .TP 8 .BI \-keyboard " keyboard-name" Use the xorg.conf(__filemansuffix__) file diff --git a/hw/xfree86/os-support/linux/systemd-logind.c b/hw/xfree86/os-support/linux/systemd-logind.c index 4ad41a374..69e2f673d 100644 --- a/hw/xfree86/os-support/linux/systemd-logind.c +++ b/hw/xfree86/os-support/linux/systemd-logind.c @@ -34,6 +34,7 @@ #include "os.h" #include "dbus-core.h" +#include "linux.h" #include "xf86.h" #include "xf86platformBus.h" #include "xf86Xinput.h" @@ -596,6 +597,13 @@ static struct dbus_core_hook core_hook = { int systemd_logind_init(void) { + if (linux_parse_vt_settings(TRUE) && !linux_get_keeptty()) { + LogMessage(X_INFO, + "systemd-logind: logind integration requires -keeptty and " + "-keeptty was not provided, disabling logind integration\n"); + return 1; + } + return dbus_core_add_hook(&core_hook); } From d3352d098724e96be8fb8538a3640ce271d8f8b9 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 22 Jun 2015 13:51:21 +0200 Subject: [PATCH 4/5] Xorg.wrap.man: Fix spelling mistake in Xorg.wrap.1 This commit fixes a small mistake in Xorg.wrap.1 . Signed-off-by: Hans de Goede Reviewed-by: Alan Coopersmith --- hw/xfree86/man/Xorg.wrap.man | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/xfree86/man/Xorg.wrap.man b/hw/xfree86/man/Xorg.wrap.man index 11090f1f4..f3f30a6c4 100644 --- a/hw/xfree86/man/Xorg.wrap.man +++ b/hw/xfree86/man/Xorg.wrap.man @@ -55,7 +55,7 @@ The default is \fIconsole\fP. \fBneeds_root_rights\fP = \fIyes\fP|\fIno\fP|\fIauto\fP Configure if the wrapper should drop its elevated (root) rights before starting the X server. Use \fIyes\fP to force execution as root, \fIno\fP to force -execution with all suid rights dropped, and \fIauto\fP to letter the wrapper +execution with all suid rights dropped, and \fIauto\fP to let the wrapper auto-detect. The default is \fIauto\fP. .PP When auto-detecting the wrapper will drop rights if kms graphics are available From 780a69aff0ff57ef813179a2f09556eaf488cdfc Mon Sep 17 00:00:00 2001 From: David Herrmann Date: Mon, 22 Jun 2015 21:13:05 +0200 Subject: [PATCH 5/5] systemd-logind: do not rely on directed signals Right now, Xorg does not install DBus matches for "PauseDevice" / "ResumeDevice". Therefore, it should usually not receive those DBus signals from logind. It is just a coincidence that systemd-logind sends those signals in a directed manner right now. Therefore, dbus-daemon bypasses the broadcast matches. However, this is not ABI and Xorg should not rely on this. systemd-logind is free to send those signals as broadcasts, in which case Xorg will freeze the VT. Fix this by always installing those matches. Cc: Hans de Goede Cc: Keith Packard Reported-by: Jan Alexander Steffens Signed-off-by: David Herrmann Reviewed-by: Hans de Goede Tested-by: Hans de Goede Signed-off-by: Hans de Goede --- hw/xfree86/os-support/linux/systemd-logind.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/hw/xfree86/os-support/linux/systemd-logind.c b/hw/xfree86/os-support/linux/systemd-logind.c index 69e2f673d..2612d9e23 100644 --- a/hw/xfree86/os-support/linux/systemd-logind.c +++ b/hw/xfree86/os-support/linux/systemd-logind.c @@ -507,6 +507,24 @@ connect_hook(DBusConnection *connection, void *data) goto cleanup; } + dbus_bus_add_match(connection, + "type='signal',sender='org.freedesktop.login1',interface='org.freedesktop.login1.Session',member='PauseDevice'", + &error); + if (dbus_error_is_set(&error)) { + LogMessage(X_ERROR, "systemd-logind: could not add match: %s\n", + error.message); + goto cleanup; + } + + dbus_bus_add_match(connection, + "type='signal',sender='org.freedesktop.login1',interface='org.freedesktop.login1.Session',member='ResumeDevice'", + &error); + if (dbus_error_is_set(&error)) { + LogMessage(X_ERROR, "systemd-logind: could not add match: %s\n", + error.message); + goto cleanup; + } + /* * HdG: This is not useful with systemd <= 208 since the signal only * contains invalidated property names there, rather than property, val