Hi all,
This patchset enables shmoverride/qubes-guid daemon support
for running multiple X servers concurrently. It allows
having one (or some) appvm GUIs under X server(s) that
runs under other virtual console. The need for this has
come up at mailing list from time to time, e.g., opening
windows VM in a second virtual console [1] or an ability to
connect appvm directly to login screen by having a custom
xsession for that [2]. The custom xsession should also allow
limited multi-user support (however, Qubes RPC support for
that kind of usecase is currently very limited/does not
exits for more than one X server but denying cross-user
RPCs manually might be enough for start).
If anyone has any thought / directions on how / what
should be implement for better Qubes RPC support for
the multi-user/X server case, please share.
This patchset is tested in R3.2. There are two patches for
gui-daemon (the first is just to introduce a variable for
shm.id filename and the second is for the actual logic changes).
The gui-daemon changes in this patchset depends on 3c86bef963
(Move /var/run/shm.id to /var/run/qubes/shm.id) which
is not in R3.2 (I cherrypicked it to R3.2 to make master
integration easier). The DISPLAY value is extracted from
/proc/self/cmdline for the shmoverride constructor which
I hope is an acceptable solution. I'm unsure what the code
should do if there is no DISPLAY found on the cmdline,
perhaps it should just bail out instead of returning :0
as it does in the current changeset?
There are two patches for core-admin. The second patch
for core-admin is not strictly necessary for R3.2 but
in order to make it easier to turn it master compatible
I added the second cludge change for testing in a more
master like codebase that supports migration from one
shm.id path to another. If requested, I can provide the
master based patch with the end result, however, testing
with master/R4.0 does not seem realistic for me at the
moment (is that required?).
--
i.
[1] https://groups.google.com/forum/#!topic/qubes-devel/6inz6v2ByNo
[2] https://groups.google.com/forum/#!msg/qubes-devel/XnTly2JNfPw/BppN_D5vn4kJ
From c9c082603360e081b671f0d655d41b8014583f60 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <[email protected]>
Date: Fri, 9 Dec 2016 00:15:42 +0200
Subject: [PATCH 1/2] Move shm.id file to variables
Should introduce no functional changes
---
gui-daemon/xside.c | 9 ++++++---
gui-daemon/xside.h | 1 -
include/shmid.h | 23 +++++++++++++++++++++++
shmoverride/shmoverride.c | 28 ++++++++++++++++++----------
4 files changed, 47 insertions(+), 14 deletions(-)
create mode 100644 include/shmid.h
diff --git a/gui-daemon/xside.c b/gui-daemon/xside.c
index 4b2bc99..e5770fc 100644
--- a/gui-daemon/xside.c
+++ b/gui-daemon/xside.c
@@ -56,6 +56,7 @@
#include "error.h"
#include "png.h"
#include "trayicon.h"
+#include "shmid.h"
/* some configuration */
@@ -3146,6 +3147,7 @@ int main(int argc, char **argv)
int pipe_notify[2];
char dbg_log[256];
char dbg_log_old[256];
+ char shmid_filename[SHMID_FILENAME_LEN];
int logfd;
char cmd_tmp[256];
struct stat stat_buf;
@@ -3157,6 +3159,7 @@ int main(int argc, char **argv)
parse_config(&ghandles);
/* parse cmdline, possibly overriding values from config */
parse_cmdline(&ghandles, argc, argv);
+ snprintf(shmid_filename, SHMID_FILENAME_LEN, SHMID_FILENAME);
get_boot_lock(ghandles.domid);
if (!ghandles.nofork) {
@@ -3179,11 +3182,11 @@ int main(int argc, char **argv)
// inside the daemonized process...
if (!ghandles.invisible) {
- f = fopen(GUID_SHMID_FILE, "r");
+ f = fopen(shmid_filename, "r");
if (!f) {
fprintf(stderr,
"Missing %s; run X with preloaded shmoverride\n",
- GUID_SHMID_FILE);
+ shmid_filename);
exit(1);
}
fscanf(f, "%d", &ghandles.cmd_shmid);
@@ -3193,7 +3196,7 @@ int main(int argc, char **argv)
fprintf(stderr,
"Invalid or stale shm id 0x%x in %s\n",
ghandles.cmd_shmid,
- GUID_SHMID_FILE);
+ shmid_filename);
exit(1);
}
}
diff --git a/gui-daemon/xside.h b/gui-daemon/xside.h
index 819f005..7659c45 100644
--- a/gui-daemon/xside.h
+++ b/gui-daemon/xside.h
@@ -30,7 +30,6 @@
#define QREXEC_POLICY_PATH "/usr/lib/qubes/qrexec-policy"
#define GUID_CONFIG_FILE "/etc/qubes/guid.conf"
#define GUID_CONFIG_DIR "/etc/qubes"
-#define GUID_SHMID_FILE "/var/run/qubes/shm.id"
/* this makes any X11 error fatal (i.e. cause exit(1)). This behavior was the
* case for a long time before introducing this option, so nothing really have
* changed */
diff --git a/include/shmid.h b/include/shmid.h
new file mode 100644
index 0000000..d4def66
--- /dev/null
+++ b/include/shmid.h
@@ -0,0 +1,23 @@
+/*
+ * The Qubes OS Project, http://www.qubes-os.org
+ *
+ * Copyright (C) 2016 Ilpo Järvinen <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program 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 General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ */
+
+#define SHMID_FILENAME "/var/run/qubes/shm.id"
+#define SHMID_FILENAME_LEN (sizeof(SHMID_FILENAME_PREFIX))
diff --git a/shmoverride/shmoverride.c b/shmoverride/shmoverride.c
index 494eaf2..e71412b 100644
--- a/shmoverride/shmoverride.c
+++ b/shmoverride/shmoverride.c
@@ -37,6 +37,7 @@
#include <errno.h>
#include <unistd.h>
#include "list.h"
+#include "shmid.h"
#include <qubes-gui-protocol.h>
static void *(*real_shmat) (int shmid, const void *shmaddr, int shmflg);
@@ -52,6 +53,8 @@ static xc_interface *xc_hnd;
static int xc_hnd;
#endif
static int list_len;
+static char __shmid_filename[SHMID_FILENAME_LEN];
+static char *shmid_filename = NULL;
void *shmat(int shmid, const void *shmaddr, int shmflg)
{
@@ -116,7 +119,6 @@ int shmctl(int shmid, int cmd, struct shmid_ds *buf)
return 0;
}
-#define SHMID_FILENAME "/var/run/qubes/shm.id"
int __attribute__ ((constructor)) initfunc()
{
int idfd, len;
@@ -141,9 +143,12 @@ int __attribute__ ((constructor)) initfunc()
perror("shmoverride xc_interface_open");
return 0; //allow it to run when not under Xen
}
- idfd = open(SHMID_FILENAME, O_WRONLY | O_CREAT | O_EXCL, 0600);
+ snprintf(__shmid_filename, SHMID_FILENAME_LEN, SHMID_FILENAME);
+ shmid_filename = __shmid_filename;
+ idfd = open(shmid_filename, O_WRONLY | O_CREAT | O_EXCL, 0600);
if (idfd < 0) {
- perror("shmoverride creating " SHMID_FILENAME);
+ fprintf(stderr, "shmoverride creating %s: %s\n",
+ shmid_filename, strerror(errno));
xc_interface_close(xc_hnd);
return 0;
}
@@ -151,25 +156,27 @@ int __attribute__ ((constructor)) initfunc()
shmget(IPC_PRIVATE, SHM_CMD_NUM_PAGES * 4096,
IPC_CREAT | 0700);
if (local_shmid == -1) {
- unlink(SHMID_FILENAME);
+ unlink(shmid_filename);
perror("shmoverride shmget");
exit(1);
}
sprintf(idbuf, "%d", local_shmid);
len = strlen(idbuf);
if (write(idfd, idbuf, len) != len) {
- unlink(SHMID_FILENAME);
- perror("shmoverride writing " SHMID_FILENAME);
+ unlink(shmid_filename);
+ fprintf(stderr, "shmoverride writing %s: %s\n",
+ shmid_filename, strerror(errno));
exit(1);
}
if (close(idfd) < 0) {
- unlink(SHMID_FILENAME);
- perror("shmoverride closing " SHMID_FILENAME);
+ unlink(shmid_filename);
+ fprintf(stderr, "shmoverride closing %s: %s\n",
+ shmid_filename, strerror(errno));
exit(1);
}
cmd_pages = real_shmat(local_shmid, 0, 0);
if (!cmd_pages) {
- unlink(SHMID_FILENAME);
+ unlink(shmid_filename);
perror("real_shmat");
exit(1);
}
@@ -182,7 +189,8 @@ int __attribute__ ((destructor)) descfunc()
if (cmd_pages) {
real_shmdt(cmd_pages);
real_shmctl(local_shmid, IPC_RMID, 0);
- unlink(SHMID_FILENAME);
+ if (shmid_filename != NULL)
+ unlink(shmid_filename);
}
return 0;
}
--
2.5.5
From 45dfeac322cbe07a864db81f1757b1d315401772 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <[email protected]>
Date: Fri, 9 Dec 2016 00:40:35 +0200
Subject: [PATCH 2/2] Use shm.id.$DISPLAY to allow more than single X server
---
gui-daemon/xside.c | 9 +++++++-
gui-daemon/xside.h | 2 +-
include/shmid.h | 6 ++++--
shmoverride/README | 6 +++---
shmoverride/shmoverride.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++-
5 files changed, 68 insertions(+), 8 deletions(-)
diff --git a/gui-daemon/xside.c b/gui-daemon/xside.c
index e5770fc..3114e88 100644
--- a/gui-daemon/xside.c
+++ b/gui-daemon/xside.c
@@ -3151,6 +3151,7 @@ int main(int argc, char **argv)
int logfd;
char cmd_tmp[256];
struct stat stat_buf;
+ char *display_str;
load_default_config_values(&ghandles);
/* get the VM name to read the right section in config file */
@@ -3159,7 +3160,6 @@ int main(int argc, char **argv)
parse_config(&ghandles);
/* parse cmdline, possibly overriding values from config */
parse_cmdline(&ghandles, argc, argv);
- snprintf(shmid_filename, SHMID_FILENAME_LEN, SHMID_FILENAME);
get_boot_lock(ghandles.domid);
if (!ghandles.nofork) {
@@ -3182,6 +3182,13 @@ int main(int argc, char **argv)
// inside the daemonized process...
if (!ghandles.invisible) {
+ display_str = getenv("DISPLAY");
+ if (display_str == NULL) {
+ fprintf(stderr, "No DISPLAY in environment\n");
+ exit(1);
+ }
+ snprintf(shmid_filename, SHMID_FILENAME_LEN,
+ SHMID_FILENAME_PREFIX "%s", display_str);
f = fopen(shmid_filename, "r");
if (!f) {
fprintf(stderr,
diff --git a/gui-daemon/xside.h b/gui-daemon/xside.h
index 7659c45..485eb02 100644
--- a/gui-daemon/xside.h
+++ b/gui-daemon/xside.h
@@ -131,7 +131,7 @@ struct _global_handles {
Atom frame_extents; /* Atom: _NET_FRAME_EXTENTS */
/* shared memory handling */
struct shm_cmd *shmcmd; /* shared memory with Xorg */
- uint32_t cmd_shmid; /* shared memory id - received from shmoverride.so through shm.id file */
+ uint32_t cmd_shmid; /* shared memory id - received from shmoverride.so through shm.id.$DISPLAY file */
int inter_appviewer_lock_fd; /* FD of lock file used to synchronize shared memory access */
/* Client VM parameters */
libvchan_t *vchan;
diff --git a/include/shmid.h b/include/shmid.h
index d4def66..c0d8840 100644
--- a/include/shmid.h
+++ b/include/shmid.h
@@ -19,5 +19,7 @@
*
*/
-#define SHMID_FILENAME "/var/run/qubes/shm.id"
-#define SHMID_FILENAME_LEN (sizeof(SHMID_FILENAME_PREFIX))
+
+#define SHMID_DISPLAY_MAXLEN 20
+#define SHMID_FILENAME_PREFIX "/var/run/qubes/shm.id."
+#define SHMID_FILENAME_LEN (sizeof(SHMID_FILENAME_PREFIX) + SHMID_DISPLAY_MAXLEN)
diff --git a/shmoverride/README b/shmoverride/README
index e2a7300..98547be 100644
--- a/shmoverride/README
+++ b/shmoverride/README
@@ -5,9 +5,9 @@ instead of attaching regular shared memory, memory from a foreign domain is
attached via xc_map_foreign_pages. This mechanism is used to map composition
buffers from a foreign domain into Xorg server.
During its init, shmoverride.so creates a shared memory segment
-(cmd_pages) and writes its shmid to /var/run/qubes/shm.id. All instances of
-qubes_guid map this segment and communicate with shmoverride.so code by
-setting its fields. When qubes_guid wants its
+(cmd_pages) and writes its shmid to /var/run/qubes/shm.id.$DISPLAY. All
+instances of qubes_guid map this segment and communicate with shmoverride.so
+code by setting its fields. When qubes_guid wants its
XShmAttach(...synth_shmid...) call to be handled by shmoverride.so, it sets the
"shmid" field in the cmd_pages shared memory segment to synth_shmid just
before calling XShmAttach. Function shmat (implemented in shmoverride.so)
diff --git a/shmoverride/shmoverride.c b/shmoverride/shmoverride.c
index e71412b..a196b6b 100644
--- a/shmoverride/shmoverride.c
+++ b/shmoverride/shmoverride.c
@@ -55,6 +55,7 @@ static int xc_hnd;
static int list_len;
static char __shmid_filename[SHMID_FILENAME_LEN];
static char *shmid_filename = NULL;
+static char display_str[SHMID_DISPLAY_MAXLEN+1] = "";
void *shmat(int shmid, const void *shmaddr, int shmflg)
{
@@ -119,6 +120,52 @@ int shmctl(int shmid, int cmd, struct shmid_ds *buf)
return 0;
}
+int get_display()
+{
+ int fd;
+ ssize_t res;
+ char ch;
+ int in_arg = 0;
+
+ fd = open("/proc/self/cmdline", O_RDONLY);
+ if (fd < 0) {
+ perror("cmdline open");
+ return -1;
+ }
+
+ while(1) {
+ res = read(fd, &ch, 1);
+ if (res < 0) {
+ perror("cmdline read");
+ return -1;
+ }
+ if (res == 0)
+ break;
+
+ if (in_arg == 0 && ch != ':')
+ in_arg = -1;
+ if (in_arg >= 0) {
+ if (in_arg >= SHMID_DISPLAY_MAXLEN)
+ break;
+ if (in_arg > 0 && (ch < '0' || ch > '9'))
+ break;
+ display_str[in_arg++] = ch;
+ display_str[in_arg] = '\0';
+ }
+ if (ch == '\0')
+ in_arg = 0;
+ }
+ close(fd);
+
+ if (display_str[0] != ':') {
+ display_str[0] = ':';
+ display_str[1] = '0';
+ display_str[2] = '\0';
+ }
+
+ return 0;
+}
+
int __attribute__ ((constructor)) initfunc()
{
int idfd, len;
@@ -143,7 +190,11 @@ int __attribute__ ((constructor)) initfunc()
perror("shmoverride xc_interface_open");
return 0; //allow it to run when not under Xen
}
- snprintf(__shmid_filename, SHMID_FILENAME_LEN, SHMID_FILENAME);
+
+ if (get_display())
+ exit(1);
+ snprintf(__shmid_filename, SHMID_FILENAME_LEN,
+ SHMID_FILENAME_PREFIX "%s", display_str);
shmid_filename = __shmid_filename;
idfd = open(shmid_filename, O_WRONLY | O_CREAT | O_EXCL, 0600);
if (idfd < 0) {
--
2.5.5
From d82eb37aa92c2d2cb431080de73e2188543ba084 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <[email protected]>
Date: Mon, 19 Dec 2016 22:22:23 +0200
Subject: [PATCH 1/2] shm.id path updated to include $DISPLAY
---
core-modules/000QubesVm.py | 11 ++++++-----
core-modules/01QubesDisposableVm.py | 4 ++--
2 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/core-modules/000QubesVm.py b/core-modules/000QubesVm.py
index 8e9978a..01f1941 100644
--- a/core-modules/000QubesVm.py
+++ b/core-modules/000QubesVm.py
@@ -2012,16 +2012,17 @@ class QubesVm(object):
# Run GUI daemon in "invisible" mode, so applications started by
# prerun script will not disturb the user
extra_guid_args = ['-I']
- elif not os.path.exists('/var/run/shm.id'):
- # Start GUI daemon only when shmoverride is loaded; unless
- # preparing DispVM, where it isn't needed because of "invisible"
- # mode
- start_guid = False
if start_guid and 'DISPLAY' not in os.environ:
if verbose:
print >> sys.stderr, \
"WARNING: not starting GUI, because DISPLAY not set"
start_guid = False
+ else:
+ if not preparing_dvm and not os.path.exists('/var/run/qubes/shm.id.' + os.getenv("DISPLAY")):
+ # Start GUI daemon only when shmoverride is loaded; unless
+ # preparing DispVM, where it isn't needed because of "invisible"
+ # mode
+ start_guid = False
if start_guid:
self.start_guid(verbose=verbose, notify_function=notify_function,
diff --git a/core-modules/01QubesDisposableVm.py b/core-modules/01QubesDisposableVm.py
index 4e3ebdd..c3fb963 100644
--- a/core-modules/01QubesDisposableVm.py
+++ b/core-modules/01QubesDisposableVm.py
@@ -222,7 +222,7 @@ class QubesDisposableVm(QubesVm):
if qmemman_present:
qmemman_client.close()
- if kwargs.get('start_guid', True) and os.path.exists('/var/run/shm.id'):
+ if kwargs.get('start_guid', True) and 'DISPLAY' in os.environ and os.path.exists('/var/run/qubes/shm.id.' + os.getenv("DISPLAY")):
self.start_guid(verbose=verbose, before_qrexec=True,
notify_function=kwargs.get('notify_function', None))
@@ -230,7 +230,7 @@ class QubesDisposableVm(QubesVm):
notify_function=kwargs.get('notify_function', None))
print >>sys.stderr, "time=%s, qrexec done" % (str(time.time()))
- if kwargs.get('start_guid', True) and os.path.exists('/var/run/shm.id'):
+ if kwargs.get('start_guid', True) and 'DISPLAY' in os.environ and os.path.exists('/var/run/qubes/shm.id.' + os.getenv("DISPLAY")):
self.start_guid(verbose=verbose,
notify_function=kwargs.get('notify_function', None))
print >>sys.stderr, "time=%s, guid done" % (str(time.time()))
--
2.5.5
From 6eee39e041da02a8c6a60cfe24b4337a4804ebbd Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ilpo=20J=C3=A4rvinen?= <[email protected]>
Date: Thu, 22 Dec 2016 11:56:24 +0200
Subject: [PATCH 2/2] Match better to fallback code in master branch
---
core-modules/000QubesVm.py | 5 ++++-
core-modules/01QubesDisposableVm.py | 9 +++++++--
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/core-modules/000QubesVm.py b/core-modules/000QubesVm.py
index 01f1941..b92869b 100644
--- a/core-modules/000QubesVm.py
+++ b/core-modules/000QubesVm.py
@@ -2018,7 +2018,10 @@ class QubesVm(object):
"WARNING: not starting GUI, because DISPLAY not set"
start_guid = False
else:
- if not preparing_dvm and not os.path.exists('/var/run/qubes/shm.id.' + os.getenv("DISPLAY")):
+ if not preparing_dvm \
+ and not os.path.exists('/var/run/qubes/shm.id.' + os.getenv("DISPLAY")) \
+ and not os.path.exists('/var/run/qubes/shm.id') \
+ and not os.path.exists('/var/run/shm.id'):
# Start GUI daemon only when shmoverride is loaded; unless
# preparing DispVM, where it isn't needed because of "invisible"
# mode
diff --git a/core-modules/01QubesDisposableVm.py b/core-modules/01QubesDisposableVm.py
index c3fb963..42cb94e 100644
--- a/core-modules/01QubesDisposableVm.py
+++ b/core-modules/01QubesDisposableVm.py
@@ -40,6 +40,7 @@ except ImportError:
pass
DISPID_STATE_FILE = '/var/run/qubes/dispid'
+GUID_SHMID_FILE = ['/var/run/qubes/shm.id', '/var/run/shm.id']
class QubesDisposableVm(QubesVm):
"""
@@ -222,7 +223,9 @@ class QubesDisposableVm(QubesVm):
if qmemman_present:
qmemman_client.close()
- if kwargs.get('start_guid', True) and 'DISPLAY' in os.environ and os.path.exists('/var/run/qubes/shm.id.' + os.getenv("DISPLAY")):
+ if kwargs.get('start_guid', True) and 'DISPLAY' in os.environ and \
+ (os.path.exists('/var/run/qubes/shm.id.' + os.getenv("DISPLAY")) \
+ or any(os.path.exists(x) for x in GUID_SHMID_FILE)):
self.start_guid(verbose=verbose, before_qrexec=True,
notify_function=kwargs.get('notify_function', None))
@@ -230,7 +233,9 @@ class QubesDisposableVm(QubesVm):
notify_function=kwargs.get('notify_function', None))
print >>sys.stderr, "time=%s, qrexec done" % (str(time.time()))
- if kwargs.get('start_guid', True) and 'DISPLAY' in os.environ and os.path.exists('/var/run/qubes/shm.id.' + os.getenv("DISPLAY")):
+ if kwargs.get('start_guid', True) and 'DISPLAY' in os.environ and \
+ (os.path.exists('/var/run/qubes/shm.id.' + os.getenv("DISPLAY")) \
+ or any(os.path.exists(x) for x in GUID_SHMID_FILE)):
self.start_guid(verbose=verbose,
notify_function=kwargs.get('notify_function', None))
print >>sys.stderr, "time=%s, guid done" % (str(time.time()))
--
2.5.5
